RFR: 8346664: C2: Optimize mask check with constant offset [v3]

[Andrew Dinn]

On Wed, 29 Jan 2025 10:49:21 GMT, Matthias Ernst <duke at openjdk.org> wrote:

>> @mernst-github Thanks for the test update!
>> 
>> I see there is still discussion around this line:
>> `  return zeros > 0 && ((((jlong)1) << zeros) > mask_t->hi_as_long() && mask_t->lo_as_long() >= 0);`
>> 
>> It is indeed difficult to read. Maybe even just some clever names of temporary variables could help. But I leave that up to you ;)
>> 
>> Ping me when you want me to run testing and review again :)
>
> @eme64 After 5+ weeks I'm getting very tired of running after reviewer engagement here. I've been in this business long enough to know that leaving a drive-by comment every few days and kicking the can down the road a little further is a very effective way to signal you're not interested in this contribution. You could have "run testing" a long time ago. If you're overworked, pass it on to a different reviewer but don't string along the contributor. If you care about a contrasting experience, https://github.com/openjdk/jdk/pull/23142 with a much larger design surface took under 2 weeks because of engaged and constructive review. Maybe it might be wise for your group and you to define or revisit goals of engagement.
> 
> This is not a pet peeve feature but a really basic performance fix for foreign memory access; I want Panama to be great, but it hurts credibility if getting segment[i+1] right doesn't appear to be a priority.
> 
> Feel free to integrate this or hit the close button, I think I've done my due diligence and do not intend to engage on it any further. Thanks.

@mernst-github I'm sorry you feel that this change has taken too long to review. It certainly was delayed for some unfortunate reasons (OCA check, Christmas holidays). However, I think you are very mistaken in your comments about Emanuel's interest in your patch and commitment to getting it integrated. When he said he was juggling many other reviews that is the honest truth. His commitment to this project and the hard work that results is very much appreciated by other developers and reviewers. What Emanuel omitted to add was that he is at the same time working on committing his own changes to the compiler code base.

Far from kicking the can down the road Emanuel has actually asked you to make successive improvements to both the code and the tests that go with it. The rationale for posting such requests is something that many newer committers to this project often find difficult to understand and some think it is down to something personal but I can assure you it is not and does not mean your contribution is undervalued. The project sets very high standards for anything that gets committed and expects reviewers to ensure those standards are met. We all want the code base to be (and remain) as simple and clear as possible and the test base to provide strong guarantees that we will find errors now in newly committed code and later should the code base get updated. That is a difficult hurdle for some contributors to jump over but it is not one we are willing to lower.

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

PR Comment: https://git.openjdk.org/jdk/pull/22856#issuecomment-2621635087