RFR(s): 8038461: Test gc/g1/TestStringDeduplicationMemoryUsage.java fails with unexpected memory usage

[Per Liden]

Ok, thanks Jon!

/Per

On 04/02/2014 06:38 PM, Jon Masamitsu wrote:
>
> On 4/2/14 3:10 AM, Per Liden wrote:
>> Hi Jon,
>>
>> On 04/01/2014 11:49 PM, Jon Masamitsu wrote:
>>> Per,
>>>
>>> Is there a way you could get generateString() to return the
>>> sum of the sizes of the strings (minus 1 string)?  Instead of the doing
>>> the calculation of bytesSaved?
>>
>> Could you elaborate a bit, I'm not quite sure I understand what you
>> mean. Is there something about the bytesSaved calculation you don't like?
>
> The only thing about the calculation of bytesSaved is that I had to look at
> it (to understand how it was being done).  If the test failed and I had to
> debug the test, I would again have to look at it to understand that it
> was right.
>
> If I looked at generateString() and all it did was add up the sum of the
> size of each string that it created, it would be easier to understand.
>>
>> Each MemoryUsageTest instance could of course return the sizes of all
>> character arrays it has created (instead of just the array header
>> size), is that what you're suggesting? There would still be a
>> bytesSaved calculation inside testMemoryUsage() though, so I have a
>> feeling that's not what you meant.
>>
>> Note that generateString()/createStrings() has no real understanding
>> of whether deduplication is enabled or not and they are also used by
>> other tests, which don't care about sizes.
>
> If generateString() would return the sum of the sizes of the strings
> created,
> the caller could figure out what it wanted to do with the number (for
> example
> figure out how to decrement it to calculate the deduplication savings).
>
> If this doesn't work here or would be too messy in any way to implement,
> forget it.  The test looks right so you can consider it reviewed by me.
>
> Jon
>
>>
>> cheers,
>> /Per
>>
>>>
>>> Jon
>>>
>>> On 4/1/14 12:54 AM, Per Liden wrote:
>>>> Thanks Stefan! Need one more Review.
>>>>
>>>> /Per
>>>>
>>>> On 31 Mar 2014, at 10:49, Stefan Johansson
>>>> <stefan.johansson at oracle.com> wrote:
>>>>
>>>>> Looks good!
>>>>>
>>>>> Stefan
>>>>>
>>>>> On 2014-03-28 14:41, Per Liden wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Could I please have this test fix reviewed.
>>>>>>
>>>>>> Summary: The test in question tries to calculate the amount of
>>>>>> memory saved when using string deduplication. The problem is that
>>>>>> the JVM doing this calculation could be running without compressed
>>>>>> class pointers when the JVM doing the actual string deduplication is
>>>>>> running with compressed class pointers. When this happens the JVMs
>>>>>> will have different views on the size of an array header, which in
>>>>>> the end makes the calculation incorrect and the test thinks the
>>>>>> deduplication failed. Exactly this situation happened in a nightly
>>>>>> test run. This fix makes sure that the array header size of the JVM
>>>>>> doing the deduplication is communicated to the JVM doing the memory
>>>>>> savings calculation.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8038461
>>>>>> Webrev: http://cr.openjdk.java.net/~pliden/8038461/webrev.0/
>>>>>>
>>>>>> /Per
>>>>>>
>>>
>>
>