On Wed, 9 Mar 2022 11:00:15 GMT, David Holmes wrote:
> @tstuefe do you have some examples of (3)? I don't like introducing a fake,
> seemingly general-purpose, API for something that is very much platform
> specific. I do dislike intensely the way the ThreadWX changes pollute shared
> code, an
On Wed, 9 Mar 2022 10:11:06 GMT, Thomas Stuefe wrote:
> Call sites in shared code are now easier to mentally parse.
> os::disable_jit_calls_for_current_thread() is much clearer than
> MACOS_AARCH64_ONLY(os::ThreadWX::Enable __wx(os::ThreadWX::Write));
That's not enough, as I wrote before, the
On Wed, 9 Mar 2022 10:11:06 GMT, Thomas Stuefe wrote:
>> @parttimenerd please never force-push in an active review as it completely
>> destroys the review history and comment context!
>
> Hi @dholmes-ora , @parttimenerd
>
> I'd like to argue again for my proposal from before.
>
> All this is
On Wed, 9 Mar 2022 07:30:43 GMT, David Holmes wrote:
>> I don't know why the Linux x86 build fails.
>>
>> I tested the current version with code related to #7591 and it seems to fix
>> the remaining problems (I tested it also with NMT enabled).
>
> @parttimenerd please never force-push in an a
On Wed, 9 Mar 2022 07:28:46 GMT, David Holmes wrote:
>> But the style guide has no opinions on them...
>
> If/when the styleguide has an opinion on namespaces I would expect the same
> naming style to apply as for Classes.
>
> Hotspot is full of historical quirks like "class os" I'm afraid.
I
On Wed, 9 Mar 2022 07:01:06 GMT, Johannes Bechberger
wrote:
>> but current_thread_wx would be too? Maybe I could change both to namespaces?
>
> But the style guide has no opinions on them...
If/when the styleguide has an opinion on namespaces I would expect the same
naming style to apply as fo
On Tue, 8 Mar 2022 12:07:08 GMT, Johannes Bechberger
wrote:
>> Johannes Bechberger has refreshed the contents of this pull request, and
>> previous commits have been removed. Incremental views are not available.
>
> I don't know why the Linux x86 build fails.
>
> I tested the current version w
On Wed, 9 Mar 2022 06:49:37 GMT, Thomas Stuefe wrote:
>> But os is okay? I just use this name for grouping.
>
> "os" is okay. Its historical. Its also more of a namespace really.
but current_thread_wx would be too? Maybe I could change both to namespaces?
-
PR: https://git.openjdk.
On Wed, 9 Mar 2022 07:00:29 GMT, Johannes Bechberger
wrote:
>> "os" is okay. Its historical. Its also more of a namespace really.
>
> but current_thread_wx would be too? Maybe I could change both to namespaces?
But the style guide has no opinions on them...
-
PR: https://git.openj
On Tue, 8 Mar 2022 13:08:13 GMT, Johannes Bechberger
wrote:
>> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.hpp line 45:
>>
>>> 43: #ifdef __APPLE__
>>> 44:
>>> 45: class current_thread_wx {
>>
>> This violates the style guide for class names. It would be CurrentThreadWX -
>> but Thread
On Tue, 8 Mar 2022 12:33:10 GMT, David Holmes wrote:
>> Johannes Bechberger has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Move code to os::current_thread_wx
>> - Small fixes
>
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.hpp line
On Tue, 8 Mar 2022 10:32:46 GMT, Johannes Bechberger
wrote:
>> The WXMode for the current thread (on MacOS aarch64) is currently stored in
>> the thread class which is unnecessary as the WXMode is bound to the current
>> OS thread, not the current instance of the thread class.
>> This pull req
On Tue, 8 Mar 2022 10:32:46 GMT, Johannes Bechberger
wrote:
>> The WXMode for the current thread (on MacOS aarch64) is currently stored in
>> the thread class which is unnecessary as the WXMode is bound to the current
>> OS thread, not the current instance of the thread class.
>> This pull req
> The WXMode for the current thread (on MacOS aarch64) is currently stored in
> the thread class which is unnecessary as the WXMode is bound to the current
> OS thread, not the current instance of the thread class.
> This pull request moves the storage of the current WXMode into a thread local
>
14 matches
Mail list logo
