Thank you, Roland
On 1/7/14 2:39 AM, Roland Westrelin wrote:
http://cr.openjdk.java.net/~kvn/8028468/webrev/
New webrev:
http://cr.openjdk.java.net/~kvn/8028468/webrev.01/
Should the agent/doc/cireplay.html be updated? Is there another doc on how to
use the replay support somewhere (wiki)?
cireplay.html describes how to use SA to extract compilation replay data from
core files. Nothing changed there with my changes.
I don't think we have any wiki for compilation replay.
I added all replay command descriptions and examples into ciReplay.hpp.
ciReplay.cpp
typos:
// Use pointer because we may need to retirn inline records
// Replay Inlinig
vmError.cpp
typo:
// Do not overwite file during replay
Typos are fixed.
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;
Done.
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;
I forgot to uncomment it. It is done only when compilation (not inlining) is
replayed. It is special case. Such replaying is done by putting compilation
task on compile queue immediately after VM initialization. So we should wait
(-Xbatch, -XX:-BackgroundCompilation) until it is finished because we don't
need to execute java code (there is no program to execute). And after
compilation we exit VM:
void ciReplay::replay(TRAPS) {
int exit_code = replay_impl(THREAD);
Threads::destroy_vm();
vm_exit(exit_code);
}
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?
I added '_' to all fields in this file.
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;
Done. That code is moved to new method get_line().
Shouldn’t the fields below be private?
422 ciKlass* _iklass;
423 Method* _imethod;
424 int _entry_bci;
425 int _comp_level;
Done.
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.
Done.
I also missed _ci_inline_records field initialization so I added it to
CompileReplay constructor.
Thanks,
Vladimir
Roland.