RFR(L): 8028468: Add inlining information into ciReplay (original) (raw)

Roland Westrelin roland.westrelin at oracle.com
Wed Jan 8 00:41:13 PST 2014

Thanks for the new webrev and explanations.

ciReplay.hpp 59 // Replay data file replay_pid%p.log is also created when VM crushes

when VM crashes

That looks good to me.

Roland.

On Jan 8, 2014, at 5:02 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

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 replayinlinedata be private with an accessor? 1183 // Dump inlining replay data to the stream. 1184 void dumpinlinedata(outputStream* out); 1185 void* replayinlinedata; 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 exitcode = replayimpl(THREAD); Threads::destroyvm(); vmexit(exitcode); } Existing fields in class CompileReplay that don’t start with make the code confusing: char* bufptr; char* buffer; int bufferlength; int bufferend; int lineno; 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 >= bufferlength) { 438 int newl = bufferlength * 2; 439 char* newb = NEWRESOURCEARRAY(char, newl); 440 memcpy(newb, buffer, pos); 441 buffer = newb; 442 bufferlength = newl; 443 } Same for: 445 // null terminate it, reset the pointer and process the line 446 buffer[pos] = '\0'; 447 bufferend = pos++; 448 bufptr = buffer; Done. That code is moved to new method getline(). Shouldn’t the fields below be private? 422 ciKlass* iklass; 423 Method* imethod; 424 int entrybci; 425 int complevel; Done. I think ciInlineRecord* findciInlineRecord(Method* method, int bci, int inlinedepth) should be implemented by calling: static ciInlineRecord* findciInlineRecord(GrowableArray<ciInlineRecord*>* records, Method* method, int bci, int inlinedepth) to avoid duplication of code. Done. I also missed ciinlinerecords field initialization so I added it to CompileReplay constructor. Thanks, Vladimir Roland.

More information about the hotspot-compiler-dev mailing list