Bug 160884 - [parser] Decouple DOMException from CoreException
Summary: [parser] Decouple DOMException from CoreException
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-core (show other bugs)
Version: 4.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 4.0 M5   Edit
Assignee: Doug Schaefer CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks: 151846
  Show dependency tree
 
Reported: 2006-10-13 11:42 EDT by Jason Montojo CLA
Modified: 2008-06-20 10:41 EDT (History)
1 user (show)

See Also:


Attachments
Proposed patch (46.91 KB, patch)
2006-10-18 13:56 EDT, Jason Montojo CLA
no flags Details | Diff
Updated patch (52.62 KB, patch)
2006-10-18 14:40 EDT, Jason Montojo CLA
no flags Details | Diff
Real updated patch (47.63 KB, patch)
2006-10-18 14:52 EDT, Jason Montojo CLA
bjorn.freeman-benson: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Montojo CLA 2006-10-13 11:42:56 EDT
This bug is related to 158975 (refactor the parser to facilitate remote indexing).

Before we can put the parser into it's own JAR again, we need to decouple certain parts of it from some of the base Eclipse packages.  The first major blocker is that DOMException currently extends CoreException.  This facilitates the error logging process because CoreExceptions provide IStatus object which the loggers use.

I propose that we make DOMException extend Exception instead of CoreException.  I've gone through the code and it looks like the only part of CoreException we're actually using is getStatus() which returns an oddly-crafted IStatus object (which is created by DOMException).  It doesn't preserve the original stack trace of the error.  It just creates a new Exception and passes that along.
Comment 1 Jason Montojo CLA 2006-10-13 11:45:28 EDT
Ooops, this is actually related to 151846, not 158975.
Comment 2 Doug Schaefer CLA 2006-10-14 09:29:19 EDT
I do think the relationship between DOMException and CoreException is pretty weird. Making DOMException a regular Exception should be fine. Clients can always create their own CoreException as necessary. I do that a lot with IOExceptions in the PDOM.

This is where the move to extract the parser jar makes me uncomfortable since CoreException is the standard exception mechanism in Eclipse and moving the DOMException to Exception makes the parser less standard and, thus, harder to maintain in the long run.

So I'll agree with this request grudgingly.
Comment 3 Jason Montojo CLA 2006-10-18 13:56:36 EDT
Created attachment 52252 [details]
Proposed patch

Here's a proposed patch.  The new behaviour creates a more accurate IStatus whenever a DOMException gets logged (i.e. the stacktrace of what caused the DOMException gets retained).
Comment 4 Doug Schaefer CLA 2006-10-18 14:25:17 EDT
I couldn't apply the patch. A couple of classes have changed underneath you. Could you do an update and redo the patch.
Comment 5 Jason Montojo CLA 2006-10-18 14:40:30 EDT
Created attachment 52259 [details]
Updated patch

This one's more fresh :)
Comment 6 Jason Montojo CLA 2006-10-18 14:43:42 EDT
Comment on attachment 52259 [details]
Updated patch

Other changes made it into this patch.  Sorry about that.  I'm working on another one.
Comment 7 Jason Montojo CLA 2006-10-18 14:52:40 EDT
Created attachment 52260 [details]
Real updated patch

Some of the hacks I did to make a standalone parser JAR made it into the last patch so please don't try that one.

This patch should be clean.
Comment 8 Doug Schaefer CLA 2006-10-18 15:13:19 EDT
Thanks, looks good. Applied to HEAD.

I'm glad you submitted the earlier patch. Any implementation of ILanguage must inherit from PlatformObject. This is how we navigate from ILanguage to ILanguageUI for language specific UI customizations.

I sure wish you guys would reconsider running the parser in a headless RCP app. If RCP can be made small enough to run in cellphones (and even smaller if you don't need SWT), why isn't it good enough for mainframes?
Comment 9 Chris Recoskie CLA 2006-10-18 15:19:33 EDT
The rule we've been told is to take the resources anything uses, and multiply it by 500 users.

The OSGI jar for example is just shy of a meg.  If 500 people load that whole jar, then all total that puts a 500 meg load on the machine.  Granted this is not the greatest example, as I'm sure that not that entire jar gets loaded every single time, but you get the idea.  Even pulling in a few more megs can potentially blow things up to the point where it's unusable.
Comment 10 Doug Schaefer CLA 2006-10-18 15:26:45 EDT
Well, just be careful not to blow up any CDT features, like multi-language support.
Comment 11 Doug Schaefer CLA 2006-10-18 15:44:13 EDT
Wait a sec. So when the DOM grows to 40 meg like it does regularly during an index, that's not a concern. Where the extra meg or so behind making a headless RCP app is a problem? I usually can't index a significant project unless I use >64M of heap.
Comment 12 Chris Recoskie CLA 2006-10-18 15:46:50 EDT
(In reply to comment #11)
> Wait a sec. So when the DOM grows to 40 meg like it does regularly during an
> index, that's not a concern. Where the extra meg or so behind making a headless
> RCP app is a problem? I usually can't index a significant project unless I use
> >64M of heap.

Hmm... you've definitely got a point there.  I don't think we've been keeping good tabs on how big the DOM gets.  What's the typical source->DOM ratio?  How big a project do you need to hit that 40 meg?
Comment 13 Doug Schaefer CLA 2006-10-18 15:57:13 EDT
Well, a couple of things. First the DOM size is dependent on the size of a translation unit, not the project. If you have a lot of includes that have a lot of symbols it can reach 40 MBs. I'm not sure of the ratios but you probably want to try it out on typical projects to see how it reacts. I know I can't index Firefox without using -Xmx128M and even then GC kicks in a lot.

The other issue is PDOM. I'm not sure if you are keeping the PDOM remote but it scales with the number of symbols in the project. The Firefox source give you around 140MB PDOM and currently that is all read into memory.