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

Mon Mar 16 04:00:14 UTC 2020

Looks reasonable.

Erik

> On 16 Mar 2020, at 04:54, Jia Huang <huangjia at loongson.cn> wrote:
> 
> Hi Markus,
> 
> Is it a trivial patch and OK to be pushed?
> Or shall we wait for one more review?
> 
> Best regards,
> Jia
> 
> 在 2020/3/10 19:37, Jia Huang 写道:
>> Thanks Markus.
>> 
>> Best regards,
>> Jia
>> 
>> 在 2020/3/10 18:15, Markus Gronlund 写道:
>>> Hi Jia,
>>> 
>>> Looks like it could work.
>>> 
>>> Thanks
>>> Markus
>>> 
>>> -----Original Message-----
>>> From: Jia Huang <huangjia at loongson.cn>
>>> Sent: den 10 mars 2020 02:30
>>> To: Markus Gronlund <markus.gronlund at oracle.com>
>>> Cc: hotspot-jfr-dev at openjdk.java.net; hotspot-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
>>> 
>>> 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_v
>>>>>>> m
>>>>>>> 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
>>>>>>>>> 
> 