Bug 203662 - Perf: Unnecessary compilation when package added to second source root
Summary: Perf: Unnecessary compilation when package added to second source root
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.3.2   Edit
Assignee: Kent Johnson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-09-17 19:08 EDT by Jess Garms CLA
Modified: 2008-09-16 09:49 EDT (History)
5 users (show)

See Also:
philippe_mulet: pmc_approved+


Attachments
Patch for ignoring duplicate package creation (1.66 KB, patch)
2007-09-17 19:08 EDT, Jess Garms CLA
no flags Details | Diff
Proposed patch (1.71 KB, patch)
2007-09-27 14:46 EDT, Kent Johnson CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jess Garms CLA 2007-09-17 19:08:51 EDT
Created attachment 78597 [details]
Patch for ignoring duplicate package creation

When adding a new package to a second source root that already exists in the first, unnecessary compilation can be triggered.

To repro:

1. Create a new project with two source roots, src1 and src2.
2. Under src1, create two new classes, pkg1.pkg2.Foo and pkg1.pkg3.Bar. These should be empty classes with no references. 
3. Make sure a full build has been performed.
4. Copy a new class, pkg1.pkg3.Baz into src2. This should not require pkg1.pkg2.Foo to get recompiled and yet incremental build triggers a recompilation of Foo.

I'll attach a patch that fixes the problem in 3.3.
Comment 1 Jess Garms CLA 2007-09-25 12:31:30 EDT
Any chance of getting this in for 3.3.2? We end up hitting this case somewhat frequently in APT when files are generated, and it doubles the compile time. Thanks much.
Comment 2 Kent Johnson CLA 2007-09-25 12:32:55 EDT
Jess - exactly how slow is "double the build time" ?

For a common case, how long is an incremental build with/without the patch ?
Comment 3 Jess Garms CLA 2007-09-25 12:49:35 EDT
For one particular case, the compilation took 5 minutes without the patch, and 2.5 with it. Note that this was using a particularly slow processor, but it essentially made an incremental build into a full build.
Comment 4 Kent Johnson CLA 2007-09-25 12:55:34 EDT
Sorry was that 2.5 seconds or minutes with the patch ?

Cause if its 2.5 minutes, then we still have an issue since adding a new class to an 'empty' source folder should be very quick if the package 'exists'.
Comment 5 Jess Garms CLA 2007-09-25 13:25:17 EDT
2.5 minutes in this particular case, but almost all of that time was spent in the processor, and isn't JDT's fault.
Comment 6 Kent Johnson CLA 2007-09-27 14:46:47 EDT
Created attachment 79308 [details]
Proposed patch

Copied the package detection case from IncrementalImageBuilder findAffectedSourceFiles()
Comment 7 Kent Johnson CLA 2007-09-27 14:50:25 EDT
Released into HEAD
Comment 8 Jess Garms CLA 2007-09-27 14:51:59 EDT
Fantastic, thanks! Any chance of applying this to 3.3.2 as well?
Comment 9 Kent Johnson CLA 2007-10-01 14:10:55 EDT
Philippe - need a +1 for 3.3.2
Comment 10 Jess Garms CLA 2007-10-01 14:31:52 EDT
One question: in the updated patch, you only avoid recompilation if there is more than one source root. Is that a valid check? What if the package already exists in binary and you add a source version of the same package?
Comment 11 Kent Johnson CLA 2007-10-01 15:11:52 EDT
The call newState.isKnownPackage() only checks the packages of the source files in the project.

It doesn't know all packages on the classpath.
Comment 12 Philipe Mulet CLA 2007-10-09 09:55:32 EDT
+1 for 3.3.2
Comment 13 Kent Johnson CLA 2007-10-09 10:10:16 EDT
Released for 3.3.2
Comment 14 Frederic Fusier CLA 2007-10-29 05:49:23 EDT
Verified for 3.4M3 using I20071029-0010 build.
Comment 15 Jerome Lanneluc CLA 2007-11-21 04:53:15 EST
Doubling the compile time is very bad. This fix needs to be included in 3.3.2. Philippe please approve.
Comment 16 Jess Garms CLA 2007-11-21 12:36:34 EST
This fix was already committed to 3.3.2 by Kent on 2007-10-09. Thanks again for getting this in.
Comment 17 Philipe Mulet CLA 2007-11-22 06:47:55 EST
It is released, but wasn't officially blessed by PMC, as per 3.3.2 process.
+1 for 3.3.2
Comment 18 Eric Jodet CLA 2008-01-24 06:07:05 EST
Verified for 3.3.2 using M20080123-0800 build