Skip to content

Commit

Permalink
8224624: Inefficiencies in CodeStrings::add_comment cause timeouts
Browse files Browse the repository at this point in the history
Changing CodeStrings to a doubly-linked-list and searching for the comment with the right offset in reverse.

Reviewed-by: kvn
  • Loading branch information
TobiHartmann committed Aug 22, 2019
1 parent 0941bf8 commit 7cff981
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 9 deletions.
35 changes: 26 additions & 9 deletions src/hotspot/share/asm/codeBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1050,26 +1050,32 @@ class CodeString: public CHeapObj<mtCode> {
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);
}

bool is_comment() const { return _offset >= 0; }

public:
CodeString(const char * string, intptr_t offset = -1)
: _next(NULL), _offset(offset) {
: _next(NULL), _prev(NULL), _offset(offset) {
_string = os::strdup(string, mtCode);
}

const char * string() const { return _string; }
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()) {
Expand Down Expand Up @@ -1097,12 +1103,9 @@ CodeString* CodeStrings::find(intptr_t offset) const {

// 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;
}
Expand All @@ -1121,12 +1124,16 @@ void CodeStrings::add_comment(intptr_t offset, const char * comment) {
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
Expand All @@ -1142,8 +1149,11 @@ void CodeStrings::copy(CodeStrings& other) {
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();
}
Expand Down Expand Up @@ -1180,6 +1190,10 @@ void CodeStrings::free() {
// 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;
}
Expand All @@ -1190,6 +1204,9 @@ const char* CodeStrings::add_string(const char * string) {
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();
Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/asm/codeBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ class CodeStrings {
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
Expand All @@ -262,6 +263,7 @@ class CodeStrings {
void set_null_and_invalidate() {
#ifndef PRODUCT
_strings = NULL;
_strings_last = NULL;
#ifdef ASSERT
_defunct = true;
#endif
Expand All @@ -272,6 +274,7 @@ class CodeStrings {
CodeStrings() {
#ifndef PRODUCT
_strings = NULL;
_strings_last = NULL;
#ifdef ASSERT
_defunct = false;
#endif
Expand Down

0 comments on commit 7cff981

Please sign in to comment.