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