On Thu, 6 Mar 2025 09:48:47 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
>>> So what, @iwanowww, you say is that this PR is indeed implementation of the >>> JEP. >>> And all subtasks listed in Umbrella RFE are following up RFEs after we >>> integrated the JEP. >>> Do I understand that correctly? >> >> Yes. >> >>> Why not do what Ioi did for AOT class loading JEP? I mean, to have >>> depending PRs which are combined into one implementation push. >> >> It's definitely an option. But, most likely, there'll be some overlooked >> cases anyway (leading to additional followup RFEs). And the more convoluted >> the changes are the harder it is to validate their correctness, thus >> increasing the risks for product stability and delaying the integration. >> (I'm not sure how much time Aleksey and other contributors want to volunteer >> to this project.) >> >> Also, in case of AOT JEP the situation was quite the opposite: it started >> with a huge patch which was split into multiple mostly independent parts to >> streamline its review. For x86-32 code removal there's no such patch >> prepared yet and the complete scope of work is not clear yet. >> >> IMO the crucial part is to get the port officially retired. After that the >> rest can become a good source of starter tasks :-) > > Basically what @iwanowww said: this PR *is* the removal of x86_32 port. > > After this PR integrates, it is not possible to build x86_32, because the > core implementation of it is missing, and build system would refuse to even > try building it. So this removes x86_32 port as the feature, atomically, > matching the title and intent of the JEP. *Then*, follow-up subtasks RFE > would clean up the parts of Hotspot that were added to support various > x86_32-specific features, and are no longer needed anymore. > > Honestly, I also believed the complete PR that cleans up every dusty corner > at once would be more straight-forward. But then I tried it at > https://github.com/openjdk/jdk/pull/22567. After investing a few full days on > that draft PR, and listening to what people said about it, I firmly changed > my mind, and can conclude that singular PR or series of stacked PRs are not a > great way to go with this removal. > > The massive drawbacks of complete/stacked PR are now obvious to me: > 1. It is hard to review. The complete PR is huge, 210+ files affected. A > lot of removals are logically connected across different files, and while > they are simple in isolation, it is hard for a reviewer to separate several > cleanups in large PRs. Stacked PRs would help some, but: > 2. It accrues merge conflicts very fast. This happens even when mainline is > somewhat idle without large feature integrations. I did complete PR near New > Year holidays, and it was _already_ a headache. I expect this work to be even > harder once we are closer to RDP1. It would be even more tedious with a chain > of 10+ stacked PRs, as I got the preview of this when rebasing the stack of > atomic commits in the complete draft PR several times. > 3. It is hard to reach consensus on. Non-trivial changes require thorough > review, and cobbling together multiple non-trivial changes require > polynomially more coordination. I have seen this in Win32 port removal, so > for a large PR like that I expect multiple, week-long review and amendment > sessions. Which conspires with (1) and (2). > 4. It is easy to introduce/overlook bugs. I already did this once in a > complete PR when I accidentally removed the wrong part of C1 regalloc code, > and it started ever so slightly misbehaving. And it was not obvious, because > it was obscured by other changes in the vicinity, and it only failed one test > in tier4. This conspires with (1), (2) and (3). > 5. It would introduce a single changeset that would be hard to bisect when > things go wrong. And the things wo... Also @shipilev I'm jealous of all your code removal. :) Well done getting agreement on this change. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23906#issuecomment-2703725960
