RFR: 8267341: macos attempt_reserve_memory_at(arg1, arg2, true) failure [v5]

Gerard Ziemski gziemski at openjdk.java.net
Mon Jan 10 16:21:04 UTC 2022


On Fri, 7 Jan 2022 15:46:56 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Re-enable **release_multi_mappings** gtest on macOS
>> 
>> This test would occasionally fail on macOS, but thanks to Dan's catch, it turned out that it actually only fails on those test machines that run macOS 10.13.6 or earlier.
>> 
>> The proposed fix simply makes a single call to `os::reserve_memory()` with the `executable` flag `True`, and if that failed forces `executable`  to `False` later in the actual test code (alternatively we could have just also skipped that test portion completely, but this way we actually do something rather than nothing at all).
>> 
>> Tested manually on macOS 10.13.6 and via Mach5
>
> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
> 
>   avoid impl details in the comment, reword slightly

> _Mailing list message from [David Holmes](mailto:david.holmes at oracle.com) on [hotspot-runtime-dev](mailto:hotspot-runtime-dev at mail.openjdk.java.net):_
> 
> On 8/01/2022 1:44 am, Gerard Ziemski wrote:
> 
> > On Thu, 6 Jan 2022 22:59:53 GMT, David Holmes <dholmes at openjdk.org> wrote:
> > > > Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
> > > > rename can_alloc_executable_memory to can_reserve_executable_memory
> > > 
> > > 
> > > test/hotspot/gtest/runtime/test_os.cpp line 379:
> > > > 377: #ifdef __APPLE__
> > > > 378:   // Workaround: try reserving memory with executable flag set to True
> > > > 379:   // to figure out if such operation is supported on this macOS version
> > > 
> > > 
> > > This comment really belongs on can_reserve_executable_memory now.
> > 
> > 
> > I'd prefer to have a comment at the place of the workaround. How about changing it slightly to avoid implementation details like so:
> > // Workaround: try reserving executable memory to figure out
> > // if such operation is supported on this macOS version
> 
> I still think can_alloc_executable_memory requires a comment to explain what it is doing.
> 
> David

OK, I added a nice comment. What do you think?

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

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


More information about the hotspot-runtime-dev mailing list