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

[Ioi Lam]

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