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

[Jiangli Zhou]

On Aug 7, 2015, at 12:58 AM, Dmitry Dmitriev <dmitry.dmitriev at oracle.com> wrote:

> 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. :)

I see. :) Thanks, Dmitry.

Jiangli

> 
> 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> 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> 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/
>>>>> 
>>>>> 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/bdf9e066/attachment.htm>