changeset 56051:e686b661fa05

8224624: Inefficiencies in CodeStrings::add_comment cause timeouts Summary: Changing CodeStrings to a doubly-linked-list and searching for the comment with the right offset in reverse. Reviewed-by: kvn
author thartmann
date Thu, 22 Aug 2019 12:24:02 +0200
parents 3283cff319c8
children c2bc7b07c67a
files src/hotspot/share/asm/codeBuffer.cpp src/hotspot/share/asm/codeBuffer.hpp
diffstat 2 files changed, 29 insertions(+), 9 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/asm/codeBuffer.cpp	Thu Aug 22 12:22:02 2019 +0200
+++ b/src/hotspot/share/asm/codeBuffer.cpp	Thu Aug 22 12:24:02 2019 +0200
@@ -1050,10 +1050,11 @@
   friend class CodeStrings;
   const char * _string;
   CodeString*  _next;
+  CodeString*  _prev;
   intptr_t     _offset;
 
   ~CodeString() {
-    assert(_next == NULL, "wrong interface for freeing list");
+    assert(_next == NULL && _prev == NULL, "wrong interface for freeing list");
     os::free((void*)_string);
   }
 
@@ -1061,7 +1062,7 @@
 
  public:
   CodeString(const char * string, intptr_t offset = -1)
-    : _next(NULL), _offset(offset) {
+    : _next(NULL), _prev(NULL), _offset(offset) {
     _string = os::strdup(string, mtCode);
   }
 
@@ -1069,7 +1070,12 @@
   intptr_t     offset() const { assert(_offset >= 0, "offset for non comment?"); return _offset;  }
   CodeString* next()    const { return _next; }
 
-  void set_next(CodeString* next) { _next = next; }
+  void set_next(CodeString* next) {
+    _next = next;
+    if (next != NULL) {
+      next->_prev = this;
+    }
+  }
 
   CodeString* first_comment() {
     if (is_comment()) {
@@ -1097,12 +1103,9 @@
 
 // Convenience for add_comment.
 CodeString* CodeStrings::find_last(intptr_t offset) const {
-  CodeString* a = find(offset);
-  if (a != NULL) {
-    CodeString* c = NULL;
-    while (((c = a->next_comment()) != NULL) && (c->offset() == offset)) {
-      a = c;
-    }
+  CodeString* a = _strings_last;
+  while (a != NULL && !a->is_comment() && a->offset() > offset) {
+    a = a->_prev;
   }
   return a;
 }
@@ -1121,12 +1124,16 @@
     c->set_next(_strings);
     _strings = c;
   }
+  if (c->next() == NULL) {
+    _strings_last = c;
+  }
 }
 
 void CodeStrings::assign(CodeStrings& other) {
   other.check_valid();
   assert(is_null(), "Cannot assign onto non-empty CodeStrings");
   _strings = other._strings;
+  _strings_last = other._strings_last;
 #ifdef ASSERT
   _defunct = false;
 #endif
@@ -1142,8 +1149,11 @@
   assert(is_null(), "Cannot copy onto non-empty CodeStrings");
   CodeString* n = other._strings;
   CodeString** ps = &_strings;
+  CodeString* prev = NULL;
   while (n != NULL) {
     *ps = new CodeString(n->string(),n->offset());
+    (*ps)->_prev = prev;
+    prev = *ps;
     ps = &((*ps)->_next);
     n = n->next();
   }
@@ -1180,6 +1190,10 @@
     // unlink the node from the list saving a pointer to the next
     CodeString* p = n->next();
     n->set_next(NULL);
+    if (p != NULL) {
+      assert(p->_prev == n, "missing prev link");
+      p->_prev = NULL;
+    }
     delete n;
     n = p;
   }
@@ -1190,6 +1204,9 @@
   check_valid();
   CodeString* s = new CodeString(string);
   s->set_next(_strings);
+  if (_strings == NULL) {
+    _strings_last = s;
+  }
   _strings = s;
   assert(s->string() != NULL, "should have a string");
   return s->string();
--- a/src/hotspot/share/asm/codeBuffer.hpp	Thu Aug 22 12:22:02 2019 +0200
+++ b/src/hotspot/share/asm/codeBuffer.hpp	Thu Aug 22 12:24:02 2019 +0200
@@ -249,6 +249,7 @@
 private:
 #ifndef PRODUCT
   CodeString* _strings;
+  CodeString* _strings_last;
 #ifdef ASSERT
   // Becomes true after copy-out, forbids further use.
   bool _defunct; // Zero bit pattern is "valid", see memset call in decode_env::decode_env
@@ -262,6 +263,7 @@
   void set_null_and_invalidate() {
 #ifndef PRODUCT
     _strings = NULL;
+    _strings_last = NULL;
 #ifdef ASSERT
     _defunct = true;
 #endif
@@ -272,6 +274,7 @@
   CodeStrings() {
 #ifndef PRODUCT
     _strings = NULL;
+    _strings_last = NULL;
 #ifdef ASSERT
     _defunct = false;
 #endif