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

[Calvin Cheung]

Thanks Coleen and Ioi for taking another look.

I'll wait until the completion of tier1 - 3 testing before pushing the 
changeset.

Calvin

On 10/2/19 2:29 PM, coleen.phillimore at oracle.com wrote:
> +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
>>>>>>>>>>
>>>>>>
>>>>
>>
>