8203299: StringPoolBuffer access covered by exclusive access invariant, remove (problematic) cas operations
Reviewed-by: egahlin
--- a/src/hotspot/share/jfr/recorder/storage/jfrBuffer.hpp Fri Jun 22 11:21:34 2018 +0100
+++ b/src/hotspot/share/jfr/recorder/storage/jfrBuffer.hpp Fri Jun 22 13:20:55 2018 +0200
@@ -25,7 +25,6 @@
#ifndef SHARE_VM_JFR_RECORDER_STORAGE_JFRBUFFER_HPP
#define SHARE_VM_JFR_RECORDER_STORAGE_JFRBUFFER_HPP
-#include "jni.h"
#include "memory/allocation.hpp"
//
@@ -34,7 +33,7 @@
// u1* _pos <-- next store position
// u1* _top <-- next unflushed position
//
-// const void* _identity <<-- acquired by
+// const void* _identity <-- acquired by
//
// Must be the owner before attempting stores.
// Use acquire() and/or try_acquire() for exclusive access
--- a/src/hotspot/share/jfr/recorder/stringpool/jfrStringPool.cpp Fri Jun 22 11:21:34 2018 +0100
+++ b/src/hotspot/share/jfr/recorder/stringpool/jfrStringPool.cpp Fri Jun 22 13:20:55 2018 +0200
@@ -151,7 +151,7 @@
StringPoolWriteOp(JfrChunkWriter& writer, Thread* thread) : _writer(writer), _thread(thread), _strings_processed(0) {}
bool write(Type* buffer, const u1* data, size_t size) {
buffer->acquire(_thread); // blocking
- const u4 nof_strings_used = (const u4)buffer->string_count();
+ const uint64_t nof_strings_used = buffer->string_count();
assert(nof_strings_used > 0, "invariant");
buffer->set_string_top(buffer->string_top() + nof_strings_used);
// "size processed" for string pool buffers is the number of processed string elements
@@ -208,10 +208,10 @@
return true;
}
buffer->set_top(current_top + unflushed_size);
- const u4 nof_strings_used = buffer->string_count();
+ const uint64_t nof_strings_used = buffer->string_count();
buffer->set_string_top(buffer->string_top() + nof_strings_used);
// "size processed" for string pool buffers is the number of string elements
- _processed += nof_strings_used;
+ _processed += (size_t)nof_strings_used;
buffer->release();
return true;
}
--- a/src/hotspot/share/jfr/recorder/stringpool/jfrStringPoolBuffer.cpp Fri Jun 22 11:21:34 2018 +0100
+++ b/src/hotspot/share/jfr/recorder/stringpool/jfrStringPoolBuffer.cpp Fri Jun 22 13:20:55 2018 +0200
@@ -24,13 +24,11 @@
#include "precompiled.hpp"
#include "jfr/recorder/stringpool/jfrStringPoolBuffer.hpp"
-#include "runtime/atomic.hpp"
-#include "runtime/orderAccess.hpp"
-#include "runtime/thread.inline.hpp"
JfrStringPoolBuffer::JfrStringPoolBuffer() : JfrBuffer(), _string_count_pos(0), _string_count_top(0) {}
void JfrStringPoolBuffer::reinitialize() {
+ assert(acquired_by_self() || retired(), "invariant");
concurrent_top();
set_pos((start()));
set_string_pos(0);
@@ -39,35 +37,31 @@
}
uint64_t JfrStringPoolBuffer::string_pos() const {
- return OrderAccess::load_acquire(&_string_count_pos);
+ assert(acquired_by_self() || retired(), "invariant");
+ return _string_count_pos;
}
uint64_t JfrStringPoolBuffer::string_top() const {
- return OrderAccess::load_acquire(&_string_count_top);
+ assert(acquired_by_self() || retired(), "invariant");
+ return _string_count_top;
}
uint64_t JfrStringPoolBuffer::string_count() const {
+ assert(acquired_by_self() || retired(), "invariant");
return string_pos() - string_top();
}
void JfrStringPoolBuffer::set_string_pos(uint64_t value) {
- Atomic::store(value, &_string_count_pos);
+ assert(acquired_by_self() || retired(), "invariant");
+ _string_count_pos = value;
}
void JfrStringPoolBuffer::increment(uint64_t value) {
-#if !(defined(ARM) || defined(IA32))
- Atomic::add(value, &_string_count_pos);
-#else
- // TODO: This should be fixed in Atomic::add handling for 32-bit platforms,
- // see JDK-8203283. We workaround the absence of support right here.
- uint64_t cur, val;
- do {
- cur = Atomic::load(&_string_count_top);
- val = cur + value;
- } while (Atomic::cmpxchg(val, &_string_count_pos, cur) != cur);
-#endif
+ assert(acquired_by_self() || retired(), "invariant");
+ ++_string_count_pos;
}
void JfrStringPoolBuffer::set_string_top(uint64_t value) {
- Atomic::store(value, &_string_count_top);
+ assert(acquired_by_self() || retired(), "invariant");
+ _string_count_top = value;
}
--- a/src/hotspot/share/jfr/recorder/stringpool/jfrStringPoolBuffer.hpp Fri Jun 22 11:21:34 2018 +0100
+++ b/src/hotspot/share/jfr/recorder/stringpool/jfrStringPoolBuffer.hpp Fri Jun 22 13:20:55 2018 +0200
@@ -26,7 +26,6 @@
#define SHARE_VM_JFR_RECORDER_STRINGPOOL_JFRSTRINGPOOLBUFFER_HPP
#include "jfr/recorder/storage/jfrBuffer.hpp"
-#include "utilities/globalDefinitions.hpp"
class JfrStringPoolBuffer : public JfrBuffer {
private:
--- a/src/hotspot/share/jfr/utilities/jfrAllocation.cpp Fri Jun 22 11:21:34 2018 +0100
+++ b/src/hotspot/share/jfr/utilities/jfrAllocation.cpp Fri Jun 22 13:20:55 2018 +0200
@@ -53,8 +53,8 @@
static void add(size_t alloc_size) {
if (!JfrRecorder::is_created()) {
- const jlong total_allocated = atomic_add_jlong(alloc_size, &_allocated_bytes);
- const jlong current_live_set = atomic_add_jlong(alloc_size, &_live_set_bytes);
+ const jlong total_allocated = atomic_add_jlong((jlong)alloc_size, &_allocated_bytes);
+ const jlong current_live_set = atomic_add_jlong((jlong)alloc_size, &_live_set_bytes);
log_trace(jfr, system)("Allocation: [" SIZE_FORMAT "] bytes", alloc_size);
log_trace(jfr, system)("Total alloc [" JLONG_FORMAT "] bytes", total_allocated);
log_trace(jfr, system)("Liveset: [" JLONG_FORMAT "] bytes", current_live_set);
@@ -63,11 +63,11 @@
static void subtract(size_t dealloc_size) {
if (!JfrRecorder::is_created()) {
- const size_t total_deallocated = atomic_add_jlong(dealloc_size, &_deallocated_bytes);
- const size_t current_live_set = atomic_add_jlong(dealloc_size * -1, &_live_set_bytes);
+ const jlong total_deallocated = atomic_add_jlong((jlong)dealloc_size, &_deallocated_bytes);
+ const jlong current_live_set = atomic_add_jlong(((jlong)dealloc_size * -1), &_live_set_bytes);
log_trace(jfr, system)("Deallocation: [" SIZE_FORMAT "] bytes", dealloc_size);
- log_trace(jfr, system)("Total dealloc [" SIZE_FORMAT "] bytes", total_deallocated);
- log_trace(jfr, system)("Liveset: [" SIZE_FORMAT "] bytes", current_live_set);
+ log_trace(jfr, system)("Total dealloc [" JLONG_FORMAT "] bytes", total_deallocated);
+ log_trace(jfr, system)("Liveset: [" JLONG_FORMAT "] bytes", current_live_set);
}
}