On Thu, 6 Mar 2025 00:16:12 GMT, Vladimir Ivanov <vliva...@openjdk.org> wrote:
>> There's a wide variety of options to justify the goal of the JEP. A bare >> minimum would be to just remove x86-32 build support. And on the other side >> of the spectrum the current patch would be accompanied by all >> x86-32-specific code and all the features used exclusively by x86-32 port. >> >> During previous round of discussions I expressed my preference as keeping >> JEP implementation simple and perform all non-trivial cleanups as follow-up >> RFEs. IMO it enables swift removal (and eliminates the burden to keep x86-32 >> port alive during ongoing development work) while keeping incremental >> cleanup activities at comfortable pace. >> >> Proposed patch perfectly justifies my preference. > >> 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. I, for one, also believed the complete PR would be more straight-forward. I attempted this at at https://github.com/openjdk/jdk/pull/22567. After working on that draft PR, and listening to what people said about it, I can conclude that is 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. 2. It accrues merge conflicts very fast. This happens even when mainline is somewhat idle without large feature integrations. I expect this work to be even harder once we are closer to RDP1. 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. Which 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 would go wrong, because of (1), (4) and partially by new opportunities presented by (2). For the C1 bug I mentioned above, I was able to quickly nail it through the bisection of my stack of atomic commits. That stack would not be available once we squash the commits/PRs before the integration. So while on a surface it might look more enticing to purge everything at once, the amount of hassle we would endure is hard to justify. Doing this PR for port removal + multiple post-removal cleanups piecewise lets us reach the same final state without extra work, while doing so at leisurely pace and maintaining more convenient code history for future bug hunts. Bottom-line: Let's not make our own lives harder unnecessarily. Atomic commits FTW. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23906#issuecomment-2703337731
