One more question:
When can the code below cause an early return from process_compile()? Can that
really happen?
493 if (entry_bci != _entry_bci || comp_level != _comp_level) {
494 return;
495 }
496 const char* iklass_name =
_imethod->method_holder()->name()->as_utf8();
497 const char* imethod_name = _imethod->name()->as_utf8();
498 const char* isignature = _imethod->signature()->as_utf8();
499 const char* klass_name =
method->method_holder()->name()->as_utf8();
500 const char* method_name = method->name()->as_utf8();
501 const char* signature = method->signature()->as_utf8();
502 if (strcmp(iklass_name, klass_name) != 0 ||
503 strcmp(imethod_name, method_name) != 0 ||
504 strcmp(isignature, signature) != 0) {
505 return;
506 }
507 }
Roland.
On Jan 7, 2014, at 11:39 AM, Roland Westrelin <roland.westre...@oracle.com>
wrote:
>> http://cr.openjdk.java.net/~kvn/8028468/webrev/
>
> Should the agent/doc/cireplay.html be updated? Is there another doc on how to
> use the replay support somewhere (wiki)?
>
> ciReplay.cpp
> typos:
> // Use pointer because we may need to retirn inline records
>
> // Replay Inlinig
>
> vmError.cpp
> typo:
> // Do not overwite file during replay
>
> compile.hpp
> Shouldn’t _replay_inline_data be private with an accessor?
>
> 1183 // Dump inlining replay data to the stream.
> 1184 void dump_inline_data(outputStream* out);
> 1185 void* _replay_inline_data;
>
> ciReplay.cpp
> Why was this changed? Why was it there in the first place?
> 1052 // Make sure we don't run with background compilation
> 1053 // BackgroundCompilation = false;
>
> Existing fields in class CompileReplay that don’t start with _ make the code
> confusing:
>
> char* bufptr;
> char* buffer;
> int buffer_length;
> int buffer_end;
> int line_no;
>
> etc. Maybe it would be worthwhile to fix that as part of this change?
>
> Can buffer be a growableArray? If not factor this code and other similar code
> in a method?
> 437 if (pos + 1 >= buffer_length) {
> 438 int newl = buffer_length * 2;
> 439 char* newb = NEW_RESOURCE_ARRAY(char, newl);
> 440 memcpy(newb, buffer, pos);
> 441 buffer = newb;
> 442 buffer_length = newl;
> 443 }
>
> Same for:
> 445 // null terminate it, reset the pointer and process the line
> 446 buffer[pos] = '\0';
> 447 buffer_end = pos++;
> 448 bufptr = buffer;
>
> Shouldn’t the fields below be private?
> 422 ciKlass* _iklass;
> 423 Method* _imethod;
> 424 int _entry_bci;
> 425 int _comp_level;
>
> I think
> ciInlineRecord* find_ciInlineRecord(Method* method, int bci, int inline_depth)
> should be implemented by calling:
> static ciInlineRecord* find_ciInlineRecord(GrowableArray<ciInlineRecord*>*
> records, Method* method, int bci, int inline_depth)
> to avoid duplication of code.
>
> Roland.
