RFR: 8324683: Unify AttachListener code for Posix platforms [v3]

Sonia Zaldana Calles szaldana at openjdk.org
Fri Apr 12 14:47:43 UTC 2024


On Fri, 12 Apr 2024 08:42:09 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>>> Maybe you could take my stuff and consolidate it with yours so we can get closer to the number of lines deleted that my PR has, or even beat it :-)?
>>> 
>>> All the best, Johan
>> 
>> Hi @jdksjolen,
>> 
>> Duplicate work is always annoying, sorry. To our defense, @SoniaZaldana 's work has been going on for many weeks now out in the open.
>> 
>> I advised her to leave AIX out of the patch for several reasons:
>> 
>> When analyzing the differences, we found a number of sublteties that may break AIX, and we have no easy way to check that platform. From my time as AIX porter I remember that the attach listener framework on AIX was exceptionally brittle and took a lot of grooming to get right. 
>> 
>> Since the current AIX porters are really overworked atm and the last thing they need are regressions, I would leave the AIX code base as it is for now. Let the porters deal with this if/when they find time.  Importantly, they then also can double-check whether many of the old workarounds we added back in the early 2000's (eg those close/shutdown workarounds) are still needed in modern AIX versions.
>> 
>> Or, what do you think @TheRealMDoerr?
>> 
>> Therefore, I would rather see Sonia's patch in its current form go in.
>
>> > Maybe you could take my stuff and consolidate it with yours so we can get closer to the number of lines deleted that my PR has, or even beat it :-)?
>> > All the best, Johan
>> 
>> Hi @jdksjolen,
>> 
>> Duplicate work is always annoying, sorry. To our defense, @SoniaZaldana 's work has been going on for many weeks now out in the open.
>> 
> 
> Nothing annoying about several people having the same good idea, too bad about me not noticing Sonia's PR or mail discussion. Makes me think about what I did at the beginning of March that caused me to miss it.
> 
>> I advised her to leave AIX out of the patch for several reasons:
>> 
>> When analyzing the differences, we found a number of sublteties that may break AIX, and we have no easy way to check that platform. From my time as AIX porter I remember that the attach listener framework on AIX was exceptionally brittle and took a lot of grooming to get right.
>> 
>> Since the current AIX porters are really overworked atm and the last thing they need are regressions, I would leave the AIX code base as it is for now. Let the porters deal with this if/when they find time. Importantly, they then also can double-check whether many of the old workarounds we added back in the early 2000's (eg those close/shutdown workarounds) are still needed in modern AIX versions.
>> 
>> Or, what do you think @TheRealMDoerr?
>> 
>> Therefore, I would rather see Sonia's patch in its current form go in.
> 
> Fine by me! I do think that the credentials checks could be moved out-of-line into something like `pd_check_credentials` to remove clutter from the main code.

Thanks for the comments folks and sorry to hear about the duplicate work @jdksjolen! 

@coleenp, Happy to make the name change from Posix -> Nix if that is more appropriate. 

Are there any other changes I should be aware of before pushing the updates? I can hold off until next week if that gives the AIX folks some time to catch up with the discussion.

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

PR Comment: https://git.openjdk.org/jdk/pull/18283#issuecomment-2051900392


More information about the hotspot-runtime-dev mailing list