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

[Calvin Cheung]

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)

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