Bug 265630 - [search][perfs] Regression in testSearchPackageDeclarationsWorkspace
Summary: [search][perfs] Regression in testSearchPackageDeclarationsWorkspace
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 M6   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-20 07:14 EST by Frederic Fusier CLA
Modified: 2009-03-09 11:47 EDT (History)
1 user (show)

See Also:

Attachments
Proposed patch (1.95 KB, patch)
2009-02-23 12:42 EST, Frederic Fusier CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Fusier CLA 2009-02-20 07:14:28 EST 
Looking at I20090217-2200 performance results, it looks like a regression occurred in FSWST.testSearchPackageDeclarationsWorkspace() test:
-69.4% [±0.5] | -92.9% [±0.2] | -50.2% [±0.0]

I ran JDT/Core performance tests locally and the results I got confirm the regression...
Comment 1 Frederic Fusier CLA 2009-02-20 07:14:58 EST 
I'll investigate...
Comment 2 Frederic Fusier CLA 2009-02-23 03:29:03 EST 
Results for N20090221-2000 confirm this regression but unfortunately I cannot reproduce it while running the single test in a workspace. Elapsed Process time are similar while running the test using a v_938 and a v_939.

Yourkit CPU snapshots either do not show any obvious regression. So, I will need to use our local tests to track this issue but this will be a little more complicated to figure out what really happens...
Comment 3 Frederic Fusier CLA 2009-02-23 08:08:05 EST 
I got it... After having released fix for bug 264817, I've verified that all CharOperation.match(char[], char[], boolean) callers do not make the assumption that the pattern was lowercased in the method itself when using 'false' for the isCaseSensitive argument.

I found one problem in the method matchesWithIgnoreCase(String[],char[]) of org.eclipse.jdt.internal.core.util.Util and of course fixed it, modifying this method and the caller in NameLookup.findPackageFragments(String,boolean,boolean).

Unfortunately, while rewriting this piece of code, I forgot to handle the case where the name is the single '*' pattern. For this peculiar case, the previous code didn't concatenate the compound name and just return immediately true.

Looking at the number of method calls for this peculiar case while running the performance test, it appears that there are 63 calls the NameLookup method, implying 76280 calls to the Util.matchesWithIgnoreCase method, hence explaining the performance regression...
Comment 4 Frederic Fusier CLA 2009-02-23 12:42:47 EST 
Created attachment 126482 [details]
Proposed patch
Comment 5 Frederic Fusier CLA 2009-02-23 12:43:11 EST 
Released for 3.5M6 in HEAD stream.
Comment 6 David Audel CLA 2009-03-09 11:47:42 EDT 
Verified for 3.5M6