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

[Vladimir Kozlov]

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