RFR(xs) 8231606: _method_ordering is not set during CDS dynamic dump time

[coleen.phillimore at oracle.com]

+1 looks good!
Coleen

On 10/2/19 1:31 PM, Ioi Lam wrote:
>
>
> On 10/2/19 10:20 AM, Calvin Cheung wrote:
>>
>> On 10/1/19 10:53 PM, Ioi Lam wrote:
>>> Hi Calvin,
>>>
>>> For the test case, I found out that we can actually skip the 
>>> "address" parameter, and the VM will pick an available port.
>>>
>>>     -agentlib:jdwp=transport=dt_socket,server=y,suspend=n
>>>
>>> I think the JDWP tests need to know the port number because they 
>>> need to connect to the debuggee. But in our case, we don't need to 
>>> know the port number.
>> Thanks for finding that out. It simplifies the test.
>>>
>>> For the asserts like this:
>>>
>>>     assert(Arguments::is_dumping_archive(), "dump time only");
>>>
>>> I think it will be better to say just this, so we don't need to 
>>> invent a new message every time.
>>>
>>>     Arguments::assert_is_dumping_archive();
>>
>> I've made the change. Here's an updated webrev:
>>
>>     http://cr.openjdk.java.net/~ccheung/8231606/webrev.02/
>>
>> (Sorry, I don't have a delta webrev this time)
>
> Hi Calvin,
>
> The changes look good.
>
> Thanks
> - Ioi
>
>>
>>>
>>> Have you tried a build without precompiled header so that you don't 
>>> miss an include of arguments.hpp?
>>
>> Actually, tier1 testing builds the linux-x64-debug-nopch profile. I 
>> also built it locally without problem.
>>
>> I'll rerun tier1-3 testing.
>>
>> thanks,
>>
>> Calvin
>>
>>>
>>> Thanks
>>> - Ioi
>>>
>>> On 10/1/19 4:52 PM, Calvin Cheung wrote:
>>>>
>>>> On 10/1/19 2:12 PM, coleen.phillimore at oracle.com wrote:
>>>>>
>>>>>
>>>>> On 10/1/19 4:32 PM, Calvin Cheung wrote:
>>>>>>
>>>>>> On 10/1/19 12:53 PM, Jiangli Zhou wrote:
>>>>>>> On Tue, Oct 1, 2019 at 12:15 PM <coleen.phillimore at oracle.com> 
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> It might be a good idea to go through and add some
>>>>>>>>
>>>>>>>> public static bool is_dumping_archive() { return 
>>>>>>>> DumpSharedSpaces ||
>>>>>>>> DynamicDumpSharedSpaces); }
>>>>>>>>
>>>>>>>> And replace all of these sets of conditionals with it, since it 
>>>>>>>> appears
>>>>>>>> that this case was missed.
>>>>>>> I was going to suggest something similar with a macro. A function
>>>>>>> probably is better.
>>>>>>
>>>>>> I will add the function in arguments.hpp since it will be called 
>>>>>> in several places spanning several files.
>>>>>
>>>>> Yes, arguments.hpp seems fine.  There are a lot of these.
>>>>
>>>> Updated webrev:
>>>>
>>>> http://cr.openjdk.java.net/~ccheung/8231606/webrev.01/
>>>>
>>>> I've run CDS/AppCDS tests locally on linux-x64.
>>>>
>>>> Re-running tier1 - 3 tests.
>>>>
>>>> thanks,
>>>>
>>>> Calvin
>>>>
>>>>> thanks,
>>>>> Coleen
>>>>>>
>>>>>> Is it ok?
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> Calvin
>>>>>>
>>>>>>>
>>>>>>> Best,
>>>>>>> Jiangli
>>>>>>>> Coleen
>>>>>>>>
>>>>>>>> On 10/1/19 1:14 PM, Calvin Cheung wrote:
>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8231606
>>>>>>>>>
>>>>>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8231606/webrev.00/
>>>>>>>>>
>>>>>>>>> A small fix for setting up the _method_ordering field during CDS
>>>>>>>>> dynamic dumping.
>>>>>>>>>
>>>>>>>>> Testing: tier1 - 3.
>>>>>>>>>
>>>>>>>>> thanks,
>>>>>>>>>
>>>>>>>>> Calvin
>>>>>>>>>
>>>>>
>>>
>