RFR(S): 8152358 - code and comment cleanups found during the hunt for 8077392

[Vladimir Kozlov]

Sorry for later reply when you send updated webrev already.

You changed code in OSR_migration_begin():
-  int buf_size_words = max_locals + active_monitor_count*2;
+  int buf_size_words = max_locals + active_monitor_count * (sizeof(BasicObjectLock) / wordSize);

I am suggesting next:

+ int buf_size_words = max_locals + active_monitor_count * BasicObjectLock::size();

Thanks,
Vladimir

On 3/28/16 4:19 PM, Daniel D. Daugherty wrote:
> On 3/24/16 3:59 PM, Daniel D. Daugherty wrote:
>> On 3/24/16 3:27 PM, Vladimir Kozlov wrote:
>>> In sharedRuntime.cpp you could use BasicObjectLock::size() as you did in c1_LIRAssembler_x86.cpp
>>
>> I'll take a look. I may leave that cleanup to another pass, if
>> you would be OK with it... I'm nearing the end of my 72 hour
>> stress test cycle and don't want to repeat it... :-)
>
> Here's the C1 change:
>
> $ hg diff src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
> diff -r b9efb94d011a src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
> --- a/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp    Mon Mar 07 11:28:06 2016 -0800
> +++ b/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp    Mon Mar 28 16:12:30 2016 -0700
> @@ -312,7 +312,7 @@ void LIR_Assembler::osr_entry() {
>     Register OSR_buf = osrBufferPointer()->as_pointer_register();
>     { assert(frame::interpreter_frame_monitor_size() == BasicObjectLock::size(), "adjust code below");
>       int monitor_offset = BytesPerWord * method()->max_locals() +
> -      (2 * BytesPerWord) * (number_of_locks - 1);
> +      (BasicObjectLock::size() * BytesPerWord) * (number_of_locks - 1);
>       // SharedRuntime::OSR_migration_begin() packs BasicObjectLocks in
>       // the OSR buffer using 2 word entries: first the lock and then
>       // the oop.
>
>
> Here's the existing code in SharedRuntime::OSR_migration_begin():
>
>    // Allocate temp buffer, 1 word per local & 2 per active monitor
>    int buf_size_words = max_locals + active_monitor_count * (sizeof(BasicObjectLock) / wordSize);
>    intptr_t *buf = NEW_C_HEAP_ARRAY(intptr_t,buf_size_words, mtCode);
>
>
> so is your suggestion to change:
>
>    sizeof(BasicObjectLock) / wordSize
>
> to:
>
>    BasicObjectLock::size()
>
> or am I misunderstanding what you mean?
>
> Dan
>