On Fri, 25 Apr 2025 01:36:50 GMT, Shaojin Wen <s...@openjdk.org> wrote:
> In the Throwable::printStackTrace method, StringBuilder is created multiple > times to build String. By sharing StringBuilder to build String, object > allocation and copying are reduced. > > In the scenario without suppressed and ourCause, unused IdentityHashMap is > not created. > > Through these optimizations, the performance of `new > Exception().printStackTrace()` can be improved by about 10%. Other Reviewers may have a different opinion, mine is that this change should not be integrated. Almost every piece of code in any project has a potential to be micro optimized for some arbitrary improvement, even the JDK project too. That doesn't mean that those changes should be done. I believe that this non-stop churn to the JDK code, code which has been around long enough without any practical issues, is not an improvement but a needless distraction. This is not limited to just this PR. In general, I feel that the PRs that you have been opening and proposing as performance improvements or modernizing the code have this same general pattern. To me, it's starting to feel more and more like a cryptic coding contest. I believe the free use of Unsafe, SharedSecrets, closely handholding the Java code to please a particular current implementation of JIT compiler, the almost obsessive and never ending code changes related to micro optimizations techniques (constant folding, unchecked array access) in several of these PRs, just because it's a performance improvement, isn't a good thing for the maintainability of the JDK. That's not the only thing though. In the past you have also been asked not to pursue changes like these at least not in the current form where you open a PR with no context, then create a JBS issue, again with no context in the description. The JDK project has contribution guidelines https://openjdk.org/guide/ and contributors have a role. Being a JDK committer, which you are, means they have extra responsibilities to making sure you respect and follow those guidelines and ensure the JDK code remains maintainable. Specifically, for the changes you have been doing and proposing as optimizations like this one, I have not seen you raise a mailing list discussion (even when you have been previously asked to do so) checking whether it's OK to do such changes before raising the PR. I feel that this kind of continued behaviour is borderline abusing the JDK infrastructure and the review process. For the areas that I watch, several of the JBS issues that you file have no context, the PRs you create almost always have zero regression tests (including this one), the JBS issues don't have appropriate noreg labels, you almost never state what kind of testing you have done for your changes (running the JMH micro benchmark isn't enough). Opening a PR and then pushing reviewers to tell you why the change shouldn't be done and what's the alternate way to do that change isn't a productive way to contribute. While at it, just to be clear, this doesn't mean you change your workflow to creating a mailing list disc ussion, then going ahead and opening a PR/JBS issue. If you do create a mailing list discussion and don't see any response to it, please consider it a sign that there may not be interest in the change. Some times we do integrate similar changes without pushing back too much and I believe that's reasonable. But continued and never ending churn like this is not good. I realize this comment is harsh but this isn't the first time some of the reviewers have advised you not to contribute in this manner. In general, I request you to reconsider the way you contribute to the project. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24864#issuecomment-2831834947
