Performance degradation with lombok by snjeza · Pull Request #4713 · eclipse-jdt/eclipse.jdt.core

snjeza · 2025-12-23T16:20:05Z

Fixes #4712

iloveeclipse · 2025-12-23T16:30:05Z

Could you please provide a regression test?

snjeza · 2025-12-23T19:05:01Z

Could you please provide a regression test?

Can I start tests with the -javaagent VM argument?

iloveeclipse · 2025-12-23T19:07:54Z

Can I start tests with the -javaagent VM argument?

Should be possible to start JVM from the test with extra arguments, the tests itself don't have individual configurations AFAIK.

snjeza · 2025-12-23T19:15:34Z

@iloveeclipse ~~I will try to create a test.~~

snjeza · 2025-12-30T23:50:53Z

Could you please provide a regression test?

@iloveeclipse I have added ASTConverter18Test.testIssue4712()

eclipse-jdt-bot · 2026-02-28T00:14:51Z

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

org.eclipse.jdt.core.compiler.batch/META-INF/MANIFEST.MF
org.eclipse.jdt.core.compiler.batch/pom.xml
org.eclipse.jdt.core.tests.model/META-INF/MANIFEST.MF
org.eclipse.jdt.core.tests.model/pom.xml
org.eclipse.jdt.core/META-INF/MANIFEST.MF
org.eclipse.jdt.core/pom.xml

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From b1db6ac2c7ef67b78cafc5c6c22dfdefd0f9e153 Mon Sep 17 00:00:00 2001
From: Eclipse JDT Bot <jdt-bot@eclipse.org>
Date: Sat, 28 Feb 2026 00:14:25 +0000
Subject: [PATCH] Version bump(s) for 4.40 stream


diff --git a/org.eclipse.jdt.core.compiler.batch/META-INF/MANIFEST.MF b/org.eclipse.jdt.core.compiler.batch/META-INF/MANIFEST.MF
index 915d7eef49..f1e166fde4 100644
--- a/org.eclipse.jdt.core.compiler.batch/META-INF/MANIFEST.MF
+++ b/org.eclipse.jdt.core.compiler.batch/META-INF/MANIFEST.MF
@@ -3,7 +3,7 @@ Main-Class: org.eclipse.jdt.internal.compiler.batch.Main
 Bundle-ManifestVersion: 2
 Bundle-Name: Eclipse Compiler for Java(TM)
 Bundle-SymbolicName: org.eclipse.jdt.core.compiler.batch
-Bundle-Version: 3.45.0.qualifier
+Bundle-Version: 3.45.100.qualifier
 Bundle-ClassPath: .
 Bundle-Vendor: Eclipse.org
 Automatic-Module-Name: org.eclipse.jdt.core.compiler.batch
diff --git a/org.eclipse.jdt.core.compiler.batch/pom.xml b/org.eclipse.jdt.core.compiler.batch/pom.xml
index ce59619b2e..f0801f9261 100644
--- a/org.eclipse.jdt.core.compiler.batch/pom.xml
+++ b/org.eclipse.jdt.core.compiler.batch/pom.xml
@@ -17,7 +17,7 @@
     <version>4.40.0-SNAPSHOT</version>
   </parent>
   <artifactId>org.eclipse.jdt.core.compiler.batch</artifactId>
-  <version>3.45.0-SNAPSHOT</version>
+  <version>3.45.100-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
 
   <properties>
diff --git a/org.eclipse.jdt.core.tests.model/META-INF/MANIFEST.MF b/org.eclipse.jdt.core.tests.model/META-INF/MANIFEST.MF
index aa5c6b00cf..b7e54e66b3 100644
--- a/org.eclipse.jdt.core.tests.model/META-INF/MANIFEST.MF
+++ b/org.eclipse.jdt.core.tests.model/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jdt.core.tests.model;singleton:=true
-Bundle-Version: 3.13.700.qualifier
+Bundle-Version: 3.13.800.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: org.eclipse.jdt.core.tests,
diff --git a/org.eclipse.jdt.core.tests.model/pom.xml b/org.eclipse.jdt.core.tests.model/pom.xml
index 543b2b5542..9a5f15b809 100644
--- a/org.eclipse.jdt.core.tests.model/pom.xml
+++ b/org.eclipse.jdt.core.tests.model/pom.xml
@@ -19,7 +19,7 @@
     <relativePath>../tests-pom/</relativePath>
   </parent>
   <artifactId>org.eclipse.jdt.core.tests.model</artifactId>
-  <version>3.13.700-SNAPSHOT</version>
+  <version>3.13.800-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
 
   <properties>
diff --git a/org.eclipse.jdt.core/META-INF/MANIFEST.MF b/org.eclipse.jdt.core/META-INF/MANIFEST.MF
index 349cfc604c..a2de32def2 100644
--- a/org.eclipse.jdt.core/META-INF/MANIFEST.MF
+++ b/org.eclipse.jdt.core/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jdt.core; singleton:=true
-Bundle-Version: 3.45.0.qualifier
+Bundle-Version: 3.45.100.qualifier
 Bundle-Activator: org.eclipse.jdt.core.JavaCore
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
diff --git a/org.eclipse.jdt.core/pom.xml b/org.eclipse.jdt.core/pom.xml
index d3e8348cb1..49fe6a292a 100644
--- a/org.eclipse.jdt.core/pom.xml
+++ b/org.eclipse.jdt.core/pom.xml
@@ -17,7 +17,7 @@
     <version>4.40.0-SNAPSHOT</version>
   </parent>
   <artifactId>org.eclipse.jdt.core</artifactId>
-  <version>3.45.0-SNAPSHOT</version>
+  <version>3.45.100-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
 
   <properties>
-- 
2.53.0

Further information are available in Common Build Issues - Missing version increments.

iloveeclipse · 2026-03-02T20:12:35Z

I've just rebased on master

Copilot

Pull request overview

Addresses Eclipse JDT issue #4712 (“Performance degradation with lombok”) by avoiding expensive comment-mapping/scanning work when the AST contains synthetic/generated nodes with invalid/empty source ranges, and by tightening comment-mapper initialization.

Changes:

Skip visiting/comment-mapping for generated AST nodes (and generated siblings) in DefaultCommentMapper.
Avoid initializing DefaultCommentMapper when the compilation unit has no comments.
Add a regression test covering the reported scenario and harden the scanner against negative positions.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
`org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/DefaultCommentMapper.java`	Skips generated nodes/siblings during comment mapping to prevent pathological scanning behavior.
`org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/CompilationUnit.java`	Only initializes the comment mapper when there are actual comments to map.
`org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/dom/ASTConverter18Test.java`	Adds a regression test for issue #4712 (currently includes a time-based assertion).
`org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Scanner.java`	Guards scanning against negative `currentPosition` after an `IndexOutOfBoundsException`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-02T20:19:52Z

+		// The convertCompilationUnit method would need to take less than one second to complete.
+		assertTrue((System.currentTimeMillis() - start) < 1000);


This test asserts a hard 1s wall-clock limit for AST.convertCompilationUnit(), which is likely to be flaky on slower/loaded CI agents and can cause unrelated failures. Consider replacing the time-based assertion with a deterministic functional assertion (e.g., verify comment mapping behavior for generated nodes) or moving this into the existing performance test infrastructure / using a much more resilient threshold with clear skip/assumption rules for slow environments.

AST.convertCompilationUnit() takes less than 20 ms.

Not on a virtual build server that randomly pauses

stephan-herrmann

@snjeza strictly speaking this looks like a bug in lombok not in JDT (see my comment on the test). To avoid infinite bug ping pong I could be persuaded to accept the fix (as extra resilience) if you add comments to each changed code location explaining that we are handling illegal input specifically from lombok.

This is relevant for two reasons:

we don't want to give the impression that 0 is a normal / expected value in any of these position fields, we would otherwise start inserting millions of 0-comparisons in all of our code base.
- this also raises the question: perhaps that lombok bug could surface in many other locations in JDT as well?
people working on these code sections in the future need to know that the reason for the added checks lies outside JDT, otherwise folks might light-heartedly removed them.

stephan-herrmann · 2026-08-09T19:30:21Z

+	printlnCall.arguments = null;
+	printlnCall.sourceStart = typeDecl.sourceStart;
+	printlnCall.sourceEnd = typeDecl.sourceEnd;
+	printlnCall.nameSourcePosition = 0L;


Strictly speaking, this assignment is illegal: org.eclipse.jdt.internal.compiler.ast.MessageSend clearly is an internal class, not intended for access by clients. For that reason we can simply say that 0L is an illegal value for field nameSourcePosition, and none of our code is obliged to handle it.

stephan-herrmann · 2026-08-09T19:32:30Z

 			ASTNode sibling = parent == this.topSiblingParent ? (ASTNode) this.siblings[this.siblingPtr] : null;
-			if (sibling != null) {
+			// skip generated node - https://github.com/eclipse-jdt/eclipse.jdt.core/issues/4712
+			if (sibling != null && sibling.getLength() > 0) {


addition is not needed to pass the new test

stephan-herrmann · 2026-08-09T19:32:43Z

 			ASTNode sibling = this.topSiblingParent == node ? (ASTNode) this.siblings[this.siblingPtr] : null;
-			if (sibling != null) {
+			// skip generated node - https://github.com/eclipse-jdt/eclipse.jdt.core/issues/4712
+			if (sibling != null && sibling.getLength() > 0) {


not covered either

snjeza · 2026-08-10T20:30:29Z

@stephan-herrmann I have updated the PR.

stephan-herrmann · 2026-08-11T09:12:26Z

@stephan-herrmann I have updated the PR.

Thanks.

Could you please also comment on the changes that are not covered by your test? I think just the change in Scanner makes it pass, no? Did you observe problems in the other locations in real life?

Edit: Just now I discovered your second force-push which brings new tests. I'll look at them shortly.
In the future you might avoid such confusion by making follow-up changes in individual commits with a short description of what's changed, so I can follow the progress by looking at the list of commits. Force-push very much obfuscates the progress.

Edit2: NO, that second force push doesn't look like any new tests from you.

Please help me understand what you are doing when pushing to the PR, thanks.

snjeza · 2026-08-11T14:21:57Z

Could you please also comment on the changes that are not covered by your test?

I have added a comment to each of the four changed files:

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Scanner.java
org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/dom/ASTConverter18Test.java
org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/CompilationUnit.java
org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/DefaultCommentMapper.java

Edit2: NO, that second force push doesn't look like any new tests from you.

The PR includes only one test - ASTConverter18Test.testIssue4712

Please help me understand what you are doing when pushing to the PR, thanks.

I have done the following:

$ cd <git_projects>/eclipse.jdt.core
$ git checkout master
$ git pull --rebase upstream master
$ git checkout issue-4712
$ git rebase master
$ git apply <my_changes>
$ git commit -a --amend
$ git push origin issue-4712 -f

stephan-herrmann · 2026-08-11T15:28:45Z

Please help me understand what you are doing when pushing to the PR, thanks.

I have done the following:
$ cd <git_projects>/eclipse.jdt.core
$ git checkout master
$ git pull --rebase upstream master
$ git checkout issue-4712
$ git rebase master
$ git apply <my_changes>
$ git commit -a --amend
$ git push origin issue-4712 -f 

Thanks.
So your latest push (forced) combines a "rebase master" with your own changes. Unfortunately this makes it unnecessarily hard for me to see what were your latest changes vs. those coming from master. During reviewing I'd like to see how the change evolves - incrementally, rather than just being presented with an entirely new commit. I was never happy with the mainstream advice to use amend and force push all the way. I'd be much happier with individual commits, i.e., a new commit for every reaction to a review - so we'd have a real dialog review-change-review .... Those commits will be squashed in the end anyway.

Also there might have been a misunderstanding regarding this:

Could you please also comment on the changes that are not covered by your test? I think just the change in Scanner makes it pass, no? Did you observe problems in the other locations in real life?

By "comment on" I didn't mean to ask for additional code comments right now. First I wanted to understand, why you made some code changes that are not needed by your test case. Can you please explain?

snjeza · 2026-08-11T18:17:24Z

So your latest push (forced) combines a "rebase master" with your own changes. Unfortunately this makes it unnecessarily hard for me to see what were your latest changes vs. those coming from master.

I have only added the following comment to all the changed files:

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Scanner.java
org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/dom/ASTConverter18Test.java
org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/CompilationUnit.java
org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/DefaultCommentMapper.java

// Lombok or some other project can generate an incorrect source position.(0, 0)
// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/4712

Compared to the original PR, I haven’t changed the code.

First I wanted to understand, why you made some code changes that are not needed by your test case. Can you please explain?

I have checked the issue described on redhat-developer/vscode-java#4052 using https://github.com/ipkiss42/vscode-java-4052
It can’t be solved without all those changes.
I can't create a better test because I can't include lombok in the jdt.core tests

snjeza force-pushed the issue-4712 branch from 7eb066d to 3381929 Compare December 30, 2025 23:47

snjeza force-pushed the issue-4712 branch from 3381929 to 16ba92c Compare February 28, 2026 00:10

iloveeclipse force-pushed the issue-4712 branch from a50270a to 8cb5562 Compare March 2, 2026 20:12

iloveeclipse requested a review from Copilot March 2, 2026 20:12

Copilot started reviewing on behalf of iloveeclipse March 2, 2026 20:14 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

stephan-herrmann reviewed Aug 9, 2026

View reviewed changes

stephan-herrmann mentioned this pull request Aug 9, 2026

Java package taking 100% CPU under very specific conditions redhat-developer/vscode-java#4052

Open

snjeza force-pushed the issue-4712 branch from 8cb5562 to 6b3da83 Compare August 10, 2026 20:29

snjeza force-pushed the issue-4712 branch from 6b3da83 to 87945d5 Compare August 11, 2026 00:06

Performance degradation with lombok

2dd9570

snjeza force-pushed the issue-4712 branch from 87945d5 to 2dd9570 Compare August 11, 2026 14:17

		// The convertCompilationUnit method would need to take less than one second to complete.
		assertTrue((System.currentTimeMillis() - start) < 1000);

Uh oh!

Conversation

snjeza commented Dec 23, 2025

Uh oh!

iloveeclipse commented Dec 23, 2025

Uh oh!

snjeza commented Dec 23, 2025

Uh oh!

iloveeclipse commented Dec 23, 2025

Uh oh!

snjeza commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snjeza commented Dec 30, 2025

Uh oh!

eclipse-jdt-bot commented Feb 28, 2026

Uh oh!

iloveeclipse commented Mar 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

snjeza Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

jukzi Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

stephan-herrmann left a comment

Choose a reason for hiding this comment

Uh oh!

stephan-herrmann Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

stephan-herrmann Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

stephan-herrmann Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

snjeza commented Aug 10, 2026

Uh oh!

stephan-herrmann commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

snjeza commented Aug 11, 2026

Uh oh!

stephan-herrmann commented Aug 11, 2026

Uh oh!

snjeza commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

snjeza commented Dec 23, 2025 •

edited

Loading

stephan-herrmann commented Aug 11, 2026 •

edited

Loading