Bug 285607 - [performance] expensive inner loop in JavaSourceLookupUtil
Summary: [performance] expensive inner loop in JavaSourceLookupUtil
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.5   Edit
Hardware: PC Linux
: P3 normal with 1 vote (vote)
Target Milestone: 3.5.2   Edit
Assignee: Darin Wright CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2009-08-04 11:50 EDT by Kelly Campbell CLA
Modified: 2010-05-28 10:28 EDT (History)
5 users (show)

See Also:
Michael_Rennie: review+


Attachments
Simple patch to fix the unneccessary expensive inner loop operation. (1.39 KB, patch)
2009-08-04 11:50 EDT, Kelly Campbell CLA
john.arthorne: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kelly Campbell CLA 2009-08-04 11:50:45 EDT
Created attachment 143410 [details]
Simple patch to fix the unneccessary expensive inner loop operation.

Build ID: I20090611-1540

Steps To Reproduce:
1. Create a large Java project with many jars in the classpath (on the order of 1000-2000)
2. Add a breakpoint somwehere
3. Debug

When it hits the breakpoint, it can take around 10s to move to the line in the source, and subsequent jumps between lines are equally expensive.

More information:
Profiling showed that creating new Path objects is relatively expensive due to calculating the hashcode and other initialization. The translate+getPackageFragmentRoot methods in JavaSourceLookupUtil is O(n^2) on the number of jars in the classpath. So with 2000 jars, it will create 4 million Path objects.

A simple patch is attached which eliminates the major inner loop expense, but this should probably be re-written with some map lookups instead of loop lookups someday.
Comment 1 Darin Wright CLA 2009-09-09 12:31:50 EDT
review during 3.6
Comment 2 Rob Clevenger CLA 2009-09-09 12:36:09 EDT
(In reply to comment #1)
> review during 3.6

this is a really bad bug for us at Google. Can you please get this patch into a 3.5.1 or 3.5.2 release?
Comment 3 Darin Wright CLA 2009-09-09 12:42:18 EDT
It's too late for 3.5.1 (RC4 build for the platform is today). We can consider 3.5.2.
Comment 4 Rob Clevenger CLA 2009-09-09 12:46:33 EDT
(In reply to comment #3)
> It's too late for 3.5.1 (RC4 build for the platform is today). We can consider
> 3.5.2.

Thanks. Please do consider this for 3.5.2. It's a really nasty regression for us.
Comment 5 Darin Wright CLA 2009-09-10 13:45:13 EDT
Applied patch to HEAD. Marking as 3.5.2 candidate. Note that this is not technically a regression in 3.5 - the same code has existed since 3.2.
Comment 6 Darin Wright CLA 2009-10-15 09:01:52 EDT
Applied to 3.5 maintenance branch. Fixed.

Please verify, Mike.
Comment 7 Michael Rennie CLA 2009-10-15 10:19:33 EDT
looks good.