RFR: 8272807: Permit use of memory concurrent with pretouch [v2]

Kim Barrett kbarrett at openjdk.java.net
Fri Oct 1 00:32:40 UTC 2021


On Tue, 21 Sep 2021 23:01:08 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Note that this PR replaces the withdrawn https://github.com/openjdk/jdk/pull/5215.
>> 
>> Please review this change which adds os::touch_memory, which is similar to
>> os::pretouch_memory but allows concurrent access to the memory while it is
>> being touched.  This is accomplished by using an atomic add of zero as the
>> operation for touching the memory, ensuring the virtual location is backed
>> by physical memory while not changing any values being read or written by
>> other threads.
>> 
>> While I was there, fixed some other lurking issues in os::pretouch_memory.
>> There was a potential overflow in the iteration that has been fixed.  And if
>> the range arguments weren't page aligned then the last page might not get
>> touched.  The latter was even mentioned in the function's description.  Both
>> of those have been fixed by careful alignment and some extra checks.  The
>> resulting code is a little more complicated, but more robust and complete.
>> 
>> Similarly added TouchTask, which is similar to PretouchTask.  Again here,
>> there is some cleaning up to avoid potential overflows and such.
>> 
>> - The chunk size is computed using the page size after possible adjustment
>> for UseTransparentHugePages.  We want a chunk size that reflects the actual
>> number of touches that will be performed.
>> 
>> - The chunk claim is now done using a CAS that won't exceed the range end.
>> The old atomic-fetch-and-add and check the result, which is performed by
>> each worker thread, could lead to overflow.  The old code has a test for
>> overflow, but since pointer-arithmetic overflow is UB that's not reliable.
>> 
>> - The old calculation of num_chunks for parallel touching could also
>> potentially overflow.
>> 
>> Testing:
>> mach5 tier1-3
>
> Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Merge branch 'master' into touch_memory
>  - simplify touch_impl, using conditional on bool arg rather than template specialization
>  - touch task
>  - add touch_memory

Withdrawing this. Other folks don't like various aspects of it, and I only went down this path due to unexpected performance problems with https://github.com/openjdk/jdk/pull/5215.  Revisiting that, it might be possible to deal with the performance problems after all.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5353


More information about the hotspot-dev mailing list