RFR: 8240222: [TESTBUG] gtest/jfr/test_networkUtilization.cpp failed when the number of tests is greater than or equal to 2

[Jia Huang]

Thanks Markus.

Could you give some comments or advice on this one:
Webrev:  http://cr.openjdk.java.net/~jiahuang/8240222/webrev.00/ ?

Best regards,
Jia

在 2020/3/9 19:41, Markus Gronlund 写道:
> The interface_id assigns a unique primary key for each interface.
>
> To support GTEST=REPEAT, I think the easiest way is to add a corresponding counter in the gtest itself that mirror the production code:
>
> That is, as an example, instead of the "add_interface("eth0", 1), that hardcodes the traceid = 1, have it take a value from the corresponding counter, i.e. "add_interface("eth0", next_id++)";
>
> Thanks
> Markus
>
>
>
> -----Original Message-----
> From: Jia Huang <huangjia at loongson.cn>
> Sent: den 9 mars 2020 10:57
> To: hotspot-dev at openjdk.java.net; hotspot-jfr-dev at openjdk.java.net
> Subject: Re: RFR: 8240222: [TESTBUG] gtest/jfr/test_networkUtilization.cpp failed when the number of tests is greater than or equal to 2
>
> In addition, the static local variable "id" is assigned in the function of "new_entry", that is in share/jfr/periodic/jfrNetworkUtilization.cpp.
>
> ---------------------------------------------------------------
> share/jfr/periodic/jfrNetworkUtilization.cpp
>
> static InterfaceEntry& new_entry(const NetworkInterface* iface,
> GrowableArray<InterfaceEntry>* interfaces) {
>     assert(iface != NULL, "invariant");
>     assert(interfaces != NULL, "invariant");
>
>     // single threaded premise
>     static traceid interface_id = 0;                       // this is the static local variable
>
>     const char* name = iface->get_name();
>     assert(name != NULL, "invariant");
>
>     InterfaceEntry entry;
>     const size_t length = strlen(name);
>     entry.name = NEW_C_HEAP_ARRAY(char, length + 1, mtInternal);
>     strncpy(entry.name, name, length + 1);
>     entry.id = ++interface_id;
>     entry.bytes_in = iface->get_bytes_in();
>     entry.bytes_out = iface->get_bytes_out();
>     entry.written = false;
>     return _interfaces->at(_interfaces->append(entry));
> }
> ---------------------------------------------------------------
>
> 在 2020/3/9 15:29, Jia Huang 写道:
>> Haha, I'm also trying to email hotspot-jfr-dev.
>> Thank you, David.
>>
>> 在 2020/3/9 13:03, David Holmes 写道:
>>> Hi Jia,
>>>
>>> I see this is getting no response so have cc'd hotspot-jfr-dev as
>>> they should have a better idea of how this gtest can and cannot be used.
>>>
>>> Just because the GTest framework supports "REPEAT" it doesn't mean
>>> all tests can, or should, be repeatable.
>>>
>>> Cheers,
>>> David
>>>
>>> On 29/02/2020 1:53 pm, Jia Huang wrote:
>>>> Hi David,
>>>>
>>>> 在 2020/2/28 21:16, David Holmes 写道:
>>>>> Hi Jia,
>>>>>
>>>>> On 28/02/2020 11:08 pm, Jia Huang wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8240222
>>>>>> Webrev: http://cr.openjdk.java.net/~jiahuang/8240222/webrev.00/
>>>>>>
>>>>>> gtest/jfr/test_networkUtilization.cpp failed when the number of
>>>>>> tests is greater than or equal to 2.
>>>>>> As shown in the following code, the test failed because "traceid
>>>>>> id" is a static local variable.
>>>>>> When the test runs a second time, "id" continues to grow,
>>>>>> resulting in the return value of "i" being "_interfaces.end()".
>>>>> Interesting! I'm not at all familiar with the details of gtest and
>>>>> how it actually runs things. But I would expect that we have many
>>>>> tests that rely on starting from a cleanly initialized VM state, or
>>>>> test state, and so can't simply be repeated (it would be like
>>>>> editing an arbitrary test and putting the body of main into a
>>>>> for-loop!).
>>>> I'm sorry that my last description may be a little incomplete.
>>>> JfrTestNetworkUtilization is a group of tests,it includes four
>>>> individual tests:
>>>>     --JfrTestNetworkUtilization.RequestFunctionBasic_test_vm
>>>>     --JfrTestNetworkUtilization.RequestFunctionMultiple_test_vm
>>>>     --JfrTestNetworkUtilization.InterfaceRemoved_test_vm
>>>>     --JfrTestNetworkUtilization.InterfaceReset_test_vm
>>>>
>>>> If you only run a individual test once, such as the following
>>>> Testing, it still fails.
>>>> The reason for the failure, as I said last time, is also because of
>>>> static local variables.
>>>> In the "RequestFunctionMultiple_test_vm" test, the sequence number
>>>> of "eth0" is 2.
>>>> However, the initial value of the static local variable "id" is 0.
>>>>
>>>> Testting:
>>>>      --make run-test
>>>> TEST=gtest:JfrTestNetworkUtilization.RequestFunctionMultiple_test_vm
>>>> GTEST=REPEAT=1 CONF=release
>>>>      --make run-test
>>>> TEST=gtest:JfrTestNetworkUtilization.InterfaceRemoved_test_vm
>>>> GTEST=REPEAT=1 CONF=release
>>>>      --make run-test
>>>> TEST=gtest:JfrTestNetworkUtilization.InterfaceReset_test_vm
>>>> GTEST=REPEAT=1 CONF=release
>>>>
>>>>> So I'm not sure what the right way to fix this is:
>>>>> - don't use REPEAT?
>>>>> - declare the test as not repeatable somehow?
>>>>> - change the test to make it repeatable?
>>>> In the doc/testing.md or doc/testing.html, Gtest keywords inclue
>>>> "REPEAT", so I think this means that all GTEST tests should be able
>>>> to use the "REPEAT parameter.
>>>>
>>>>
>>>> Best regards,
>>>> Jia
>>>>
>>>>
>>>>> Cheers,
>>>>> David
>>>>>
>>>>>> ----------------------------------------------------------------------------
>>>>>>
>>>>>> test/hotspot/gtest/jfr/test_networkUtilization.cpp
>>>>>>
>>>>>> static const MockNetworkInterface& get_interface(traceid id) {
>>>>>>     std::list<MockNetworkInterface>::const_iterator i =
>>>>>> _interfaces.begin();
>>>>>>     for (; i != _interfaces.end(); ++i) {
>>>>>>       if (i->id == id) {
>>>>>>         break;
>>>>>>       }
>>>>>>     }
>>>>>>     return *i;
>>>>>> }
>>>>>> ----------------------------------------------------------------------------
>>>>>>
>>>>>>
>>>>>>
>>>>>> Testing:
>>>>>>       - make run-test TEST=gtest:JfrTestNetworkUtilization
>>>>>> GTEST=REPEAT=2 CONF=server-release
>>>>>>       - make run-test TEST=gtest:JfrTestNetworkUtilization
>>>>>> GTEST=REPEAT=2 CONF=server-fastdebug
>>>>>>
>>>>>> Thanks a lot
>>>>>>
>>>>>> Best regards,
>>>>>> Jia
>>>>>>