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
--- 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