Bug 156496 - CSSResourceEncodingDetector.getEncodingMemento() causes performance regression
Summary: CSSResourceEncodingDetector.getEncodingMemento() causes performance regression
Status: CLOSED WORKSFORME
Alias: None
Product: WTP Source Editing
Classification: WebTools
Component: wst.css (show other bugs)
Version: 1.5   Edit
Hardware: PC Windows XP
: P2 major (vote)
Target Milestone: ---   Edit
Assignee: David Williams CLA
QA Contact: David Williams CLA
URL:
Whiteboard:
Keywords: performance
Depends on: 157266
Blocks:
  Show dependency tree
 
Reported: 2006-09-07 07:19 EDT by Naomi Miyamoto CLA
Modified: 2007-04-03 11:45 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Naomi Miyamoto CLA 2006-09-07 07:19:23 EDT
In our test case (build lage project), org.eclipse.wst.sse.core.internal.model.ModelManagerImpl.createStructuredDocumentFor(IFile) shows regression compare to previous product. 

For 969 times invocation, AbstractDocumentLoader.createNewStructuredDocument(IFile) takes almost same time before (4,300 msec). While ModelManagerImpl.calculateType(IFile) takes about 3,800 msec. This method is not shown in profiler (YourKit3) before.

Most of time is consumed in CSSResourceEncodingDetector.getEncodingMemento(). (This method is called 689 times.)
Comment 1 David Williams CLA 2006-09-10 19:37:48 EDT
Do you mean this is seen with thousands of CSS files, or ... what? 
I'm trying to think of how to reproduce so I can see if small improvements
in code would lead to big improvemeents in overall time. 

Comment 2 Naomi Miyamoto CLA 2006-09-12 06:36:17 EDT
(In reply to comment #1)
> Do you mean this is seen with thousands of CSS files, or ... what? 
There are 692 css files. It is almost same as the invocation count of CSSResourceEncodingDetector.getEncodingMemento(). (689 times.)
Comment 3 hoshiura CLA 2006-09-14 02:38:12 EDT
I've opened https://bugs.eclipse.org/bugs/show_bug.cgi?id=157266 which relates to this problem. I am requesting to implement cache mechanism in ContentDescriptionManager.getDescriptionFor() method to avoid a lot of I/O access in Bug#157266. The method is called between ModelManagerImpl.createStructuredDocumentFor() and CSSResourceEncodingDetector.getEncodingMemento() in call stack. So, if Bug#157266 is resolved, it should affect to this, I think.

Comment 4 David Williams CLA 2006-09-21 03:10:13 EDT
we'll investigate solutions for next round of fixes. 
Comment 5 David Williams CLA 2006-09-24 23:15:34 EDT
Untargeting to acknowledge no fix is in hand. But, will be investigated for 1.5.2. 

Setting priority to P4 to signify that it while it was enterted as 'critical' that is has been triaged and should not "stop ship", or anything. 

It is conceptually still a "P2", and will change it back once 1.5.1 ships. 
Comment 6 Naomi Miyamoto CLA 2006-10-06 07:22:21 EDT
Hot-bug request information
1. Affiliation: IBM
2. Release you want this bug to be fixed in: 1.5.2
3. Justify why this is a hot bug and why it needs to be fixed in that release:
Performance.
Comment 7 David Williams CLA 2006-10-13 18:02:07 EDT
I have looked at this, and do not really see anything that CSS Resource Encoding is doing wrong, or different. 

From what I've gathered, the "root" issue is that some information is not cached in content describer, as you've suggested in bug 157266. And that sounds like the right path to me. I am not aware of any solutions we could make in our code, and suspect you are not either, or else you would have already submitted a patch :) 

I think too this is not a simple regression, but a direct effect of adding the new functionality you requested for "CSS in JSPs"?  (Right?) 

I would even be helpful if you have a test case you could attach. I've been meaning to create a 1000 CSS files, but haven't found the time yet. 

I suspect there's nothing we can do here, and will have to wait for 157266. 
Comment 8 David Williams CLA 2006-10-17 07:24:46 EDT
Hot bug removed, as not accepted as hot bug. Plus, since "critical" means "crashes, loss of data, etc" See https://bugs.eclipse.org/bugs/page.cgi?id=fields.html#bug_severity
I've change the severity to major. 
Comment 9 David Williams CLA 2007-02-28 00:07:20 EST
I'm resolving this as works for me, as I don't think there is any bug here, per se, but related to 157266 and the CSS JSP support being added. 

Please let me know if you do see a fix we can make. 

Comment 10 John Lanuti CLA 2007-04-03 11:45:59 EDT
Closing as part of mass query to clean up old resolved bugs in untargetted milestones.