Bug 92112 - [Themes] ColorUtils.process is expensive
Summary: [Themes] ColorUtils.process is expensive
Status: RESOLVED WONTFIX
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows 2000
: P3 normal (vote)
Target Milestone: 3.1 M7   Edit
Assignee: Kim Horne CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2005-04-20 12:59 EDT by Nick Edgar CLA
Modified: 2005-04-20 16:59 EDT (History)
1 user (show)

See Also:

Attachments
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 Nick Edgar CLA 2005-04-20 12:59:40 EDT 
build I20050413

ColorUtils.process using reflection to scan over all the fields in the SWT class.
This is called for each color extension.
If there are many of these, this will be slow.

I haven't looked at actual numbers since I'm in a broken state at the moment,
but just wanted to flag this.
Comment 1 Nick Edgar CLA 2005-04-20 13:00:19 EDT 
Maybe cache a table of appropriate COLOR_* strings to the corresponding SWT Field?
Comment 2 Kim Horne CLA 2005-04-20 14:12:41 EDT 
Sounds reasonable.
Comment 3 Kim Horne CLA 2005-04-20 15:25:32 EDT 
I've gotten Tod to profile this and I've done some System.currentTimeMillis()
testing myself and it seems like all of the work is being done in the first call
to clazz.getDeclaredFields() (taking a whopping 10ms).  Every successive call to
process() cannot be measured accurately (0ms deltas).  Marking as WONTFIX.
Comment 4 Nick Edgar CLA 2005-04-20 16:39:26 EDT 
They must be caching the list themselves, probably to make getField(String) work
efficiently.
Comment 5 Nick Edgar CLA 2005-04-20 16:39:46 EDT 
Do you really want to allow any SWT constant though?  Should we check for
particular prefixes?
Comment 6 Kim Horne CLA 2005-04-20 16:41:27 EDT 
We could check for appropriate constants but it's not a really big deal.  If
they use a bad constant they just get back COLOR_BLACK.  It's a seperate bug at
any rate...
Comment 7 Nick Edgar CLA 2005-04-20 16:59:45 EDT 
Agree it's a separate bug.  It just seems strange to allow them to specify e.g.
ARROW_DOWN.