RFR (S): 8131734: Add free_archive_regions support to G1 for -Xshared:auto

[Dmitry Dmitriev]

Hello Jiangli,

Thank you. About extra spaces - I just highlight changed lines in webrev 
and see that some lines have additional spaces at the end. :)

Dmitry

On 07.08.2015 3:40, Jiangli Zhou wrote:
> Hi Dmitry,
>
> I’ve added shared string unmapping to handle the string data 
> verification failure. I also added 'free_archive_regions()’ call 
> to FileMapInfo::unmap_string_regions(). My testing revealed some 
> issues with the new archive region free code when I forced string 
> verification failure. I’m working with Tom on the issues.
>
> Thanks,
> Jiangli
>
>
> On Aug 6, 2015, at 3:19 PM, Jiangli Zhou <jiangli.zhou at oracle.com 
> <mailto:jiangli.zhou at oracle.com>> wrote:
>
>> Hi Dmitry,
>>
>> Thank you for the feedback! You are definitely right about freeing 
>> the archived regions in those failure cases. I should have thought it 
>> more thoroughly, instead of just fixing the case that I ran into. 
>> Will exam all cases.
>>
>> I’ll change the test to incorporate your suggestion. Will fix the 
>> extra spaces before committing. I have one question, how do you 
>> detect the extra spaces from the webrev? :)
>>
>> Thanks,
>> Jiangli
>>
>> On Aug 6, 2015, at 2:59 PM, Dmitry Dmitriev 
>> <dmitry.dmitriev at oracle.com <mailto:dmitry.dmitriev at oracle.com>> wrote:
>>
>>> Hello Jiangli,
>>>
>>> I have few comments/questions.
>>>
>>> src/share/vm/memory/filemap.cpp module:
>>> 1) Should free_archive_regions also called when 
>>> verify_string_regions() returns false on line 717?
>>> 2) The same question about unmap_string_regions(). Region must be 
>>> freed when unmap_string_regions() is called?
>>> 3) Extra space at the end of the line 711.
>>>
>>> test/runtime/SharedArchiveFile/SharedStringsRunAuto.java
>>> 1) Unneeded second creating of OutputAnalyzer on line 61. Also, 
>>> probably will be better to use same scheme for OutputAnalyzer? On 
>>> lines 46-49 you not use local variable, but on lines 61-63 use local 
>>> variable.
>>>  59         OutputAnalyzer output = new OutputAnalyzer(pb.start());
>>>  60
>>>  61         output = new OutputAnalyzer(pb.start());
>>> 2) Extra space at the end of the lines 25,26,30, 51
>>>
>>> Thanks,
>>> Dmitry
>>>
>>> On 07.08.2015 0:32, Jiangli Zhou wrote:
>>>> Hi,
>>>>
>>>> Here is the runtime part of the bug fix that calls the new 
>>>> free_archive_regions() when shared string mapping fails. I also 
>>>> added a jtreg test to test shared strings with -Xshare:auto.
>>>>
>>>> http://cr.openjdk.java.net/~jiangli/8131734/webrev.00/ 
>>>> <http://cr.openjdk.java.net/%7Ejiangli/8131734/webrev.00/>
>>>>
>>>> Test:
>>>> - Tested by explicitly making the shared string mapping fail on 
>>>> linux-x64, -Xshare:auto runs without crash with the fix
>>>> - Tested with the new SharedStringsRunAuto test
>>>> - Tested with XX:+PrintNMTStatistics -XX:NativeMemoryTracking=detail
>>>>
>>>> Thanks,
>>>> Jiangli
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150807/89adf47a/attachment.htm>