8150419: Cleanup BufferNode API
authorkbarrett
Wed, 24 Feb 2016 13:18:54 -0500
changeset 36354 28bbe66a0498
parent 36353 28b633a6919d
child 36356 17e26d207802
child 36357 953ab5f53cff
child 36359 05fb53b2a533
child 36361 00aaa003d2e7
8150419: Cleanup BufferNode API Summary: Fewer public functions, cleanup allocation. Reviewed-by: tschatzl, drwhite
hotspot/src/share/vm/gc/g1/ptrQueue.cpp
hotspot/src/share/vm/gc/g1/ptrQueue.hpp
--- a/hotspot/src/share/vm/gc/g1/ptrQueue.cpp	Thu Feb 18 16:15:28 2016 +0100
+++ b/hotspot/src/share/vm/gc/g1/ptrQueue.cpp	Wed Feb 24 13:18:54 2016 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -30,6 +30,8 @@
 #include "runtime/mutexLocker.hpp"
 #include "runtime/thread.inline.hpp"
 
+#include <new>
+
 PtrQueue::PtrQueue(PtrQueueSet* qset, bool permanent, bool active) :
   _qset(qset), _buf(NULL), _index(0), _sz(0), _active(active),
   _permanent(permanent), _lock(NULL)
@@ -87,6 +89,19 @@
 }
 
 
+BufferNode* BufferNode::allocate(size_t byte_size) {
+  assert(byte_size > 0, "precondition");
+  assert(is_size_aligned(byte_size, sizeof(void**)),
+         "Invalid buffer size " SIZE_FORMAT, byte_size);
+  void* data = NEW_C_HEAP_ARRAY(char, buffer_offset() + byte_size, mtGC);
+  return new (data) BufferNode;
+}
+
+void BufferNode::deallocate(BufferNode* node) {
+  node->~BufferNode();
+  FREE_C_HEAP_ARRAY(char, node);
+}
+
 PtrQueueSet::PtrQueueSet(bool notify_when_complete) :
   _max_completed_queue(0),
   _cbl_mon(NULL), _fl_lock(NULL),
@@ -123,17 +138,23 @@
 
 void** PtrQueueSet::allocate_buffer() {
   assert(_sz > 0, "Didn't set a buffer size.");
-  MutexLockerEx x(_fl_owner->_fl_lock, Mutex::_no_safepoint_check_flag);
-  if (_fl_owner->_buf_free_list != NULL) {
-    void** res = BufferNode::make_buffer_from_node(_fl_owner->_buf_free_list);
-    _fl_owner->_buf_free_list = _fl_owner->_buf_free_list->next();
-    _fl_owner->_buf_free_list_sz--;
-    return res;
+  BufferNode* node = NULL;
+  {
+    MutexLockerEx x(_fl_owner->_fl_lock, Mutex::_no_safepoint_check_flag);
+    node = _fl_owner->_buf_free_list;
+    if (node != NULL) {
+      _fl_owner->_buf_free_list = node->next();
+      _fl_owner->_buf_free_list_sz--;
+    }
+  }
+  if (node == NULL) {
+    node = BufferNode::allocate(_sz);
   } else {
-    // Allocate space for the BufferNode in front of the buffer.
-    char *b =  NEW_C_HEAP_ARRAY(char, _sz + BufferNode::aligned_size(), mtGC);
-    return BufferNode::make_buffer_from_block(b);
+    // Reinitialize buffer obtained from free list.
+    node->set_index(0);
+    node->set_next(NULL);
   }
+  return BufferNode::make_buffer_from_node(node);
 }
 
 void PtrQueueSet::deallocate_buffer(void** buf) {
@@ -150,13 +171,13 @@
   // For now we'll adopt the strategy of deleting half.
   MutexLockerEx x(_fl_lock, Mutex::_no_safepoint_check_flag);
   size_t n = _buf_free_list_sz / 2;
-  while (n > 0) {
-    assert(_buf_free_list != NULL, "_buf_free_list_sz must be wrong.");
-    void* b = BufferNode::make_block_from_node(_buf_free_list);
-    _buf_free_list = _buf_free_list->next();
-    FREE_C_HEAP_ARRAY(char, b);
-    _buf_free_list_sz --;
-    n--;
+  for (size_t i = 0; i < n; ++i) {
+    assert(_buf_free_list != NULL,
+           "_buf_free_list_sz is wrong: " SIZE_FORMAT, _buf_free_list_sz);
+    BufferNode* node = _buf_free_list;
+    _buf_free_list = node->next();
+    _buf_free_list_sz--;
+    BufferNode::deallocate(node);
   }
 }
 
@@ -236,8 +257,9 @@
 
 void PtrQueueSet::enqueue_complete_buffer(void** buf, size_t index) {
   MutexLockerEx x(_cbl_mon, Mutex::_no_safepoint_check_flag);
-  BufferNode* cbn = BufferNode::new_from_buffer(buf);
+  BufferNode* cbn = BufferNode::make_node_from_buffer(buf);
   cbn->set_index(index);
+  cbn->set_next(NULL);
   if (_completed_buffers_tail == NULL) {
     assert(_completed_buffers_head == NULL, "Well-formedness");
     _completed_buffers_head = cbn;
--- a/hotspot/src/share/vm/gc/g1/ptrQueue.hpp	Thu Feb 18 16:15:28 2016 +0100
+++ b/hotspot/src/share/vm/gc/g1/ptrQueue.hpp	Wed Feb 24 13:18:54 2016 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,9 +33,6 @@
 // the addresses of modified old-generation objects.  This type supports
 // this operation.
 
-// The definition of placement operator new(size_t, void*) in the <new>.
-#include <new>
-
 class PtrQueueSet;
 class PtrQueue VALUE_OBJ_CLASS_SPEC {
   friend class VMStructs;
@@ -168,42 +165,38 @@
 class BufferNode {
   size_t _index;
   BufferNode* _next;
+  void* _buffer[1];             // Pseudo flexible array member.
+
+  BufferNode() : _index(0), _next(NULL) { }
+  ~BufferNode() { }
+
+  static size_t buffer_offset() {
+    return offset_of(BufferNode, _buffer);
+  }
+
 public:
-  BufferNode() : _index(0), _next(NULL) { }
   BufferNode* next() const     { return _next;  }
   void set_next(BufferNode* n) { _next = n;     }
   size_t index() const         { return _index; }
   void set_index(size_t i)     { _index = i;    }
 
-  // Align the size of the structure to the size of the pointer
-  static size_t aligned_size() {
-    static const size_t alignment = round_to(sizeof(BufferNode), sizeof(void*));
-    return alignment;
-  }
+  // Allocate a new BufferNode with the "buffer" having size bytes.
+  static BufferNode* allocate(size_t byte_size);
 
-  // BufferNode is allocated before the buffer.
-  // The chunk of memory that holds both of them is a block.
+  // Free a BufferNode.
+  static void deallocate(BufferNode* node);
 
-  // Produce a new BufferNode given a buffer.
-  static BufferNode* new_from_buffer(void** buf) {
-    return new (make_block_from_buffer(buf)) BufferNode;
+  // Return the BufferNode containing the buffer.
+  static BufferNode* make_node_from_buffer(void** buffer) {
+    return reinterpret_cast<BufferNode*>(
+      reinterpret_cast<char*>(buffer) - buffer_offset());
   }
 
-  // The following are the required conversion routines:
-  static BufferNode* make_node_from_buffer(void** buf) {
-    return (BufferNode*)make_block_from_buffer(buf);
-  }
+  // Return the buffer for node.
   static void** make_buffer_from_node(BufferNode *node) {
-    return make_buffer_from_block(node);
-  }
-  static void* make_block_from_node(BufferNode *node) {
-    return (void*)node;
-  }
-  static void** make_buffer_from_block(void* p) {
-    return (void**)((char*)p + aligned_size());
-  }
-  static void* make_block_from_buffer(void** p) {
-    return (void*)((char*)p - aligned_size());
+    // &_buffer[0] might lead to index out of bounds warnings.
+    return reinterpret_cast<void**>(
+      reinterpret_cast<char*>(node) + buffer_offset());
   }
 };