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

[Roland Westrelin]

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.westrelin at 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.