This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 13847 - Rewrite NbBundle to not use ResourceBundle
Summary: Rewrite NbBundle to not use ResourceBundle
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: -- Other -- (show other bugs)
Version: 3.x
Hardware: PC Linux
: P2 blocker (vote)
Assignee: Jesse Glick
URL:
Keywords: PERFORMANCE
Depends on: 15689 31034 31578
Blocks: 18310
  Show dependency tree
 
Reported: 2001-07-23 19:03 UTC by Jesse Glick
Modified: 2008-12-23 13:48 UTC (History)
3 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
Useless patch using binary ".cproperties" format (4.96 KB, patch)
2003-02-04 02:12 UTC, Jesse Glick
Details | Diff
More successful patch (7.56 KB, patch)
2003-02-04 04:01 UTC, Jesse Glick
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2001-07-23 19:03:19 UTC
Rewrite NbBundle to load resource bundles without help from ResourceBundle. This
would permit (1) faster loading since we can skip .class loading, or check this
as a last resort perhaps; (2) more direct algorithm for finding branded
variants; (3) remove unnecessary search for useless locales like en_US; (4)
avoid a reported but unfixed caching bug in ResourceBundle (SoftReference
instead of WeakReference) which makes modules not always release resources when
uninstalled.
Comment 1 Jan Chalupa 2001-11-27 13:03:24 UTC
Target milestone -> 3.3.1.
Comment 2 Petr Nejedly 2001-12-03 17:25:28 UTC
It would also be fine if the new implementation provide either
boolean hasKey(String key) or (probably better)
String getStringOrNull(String key) /* throws nothing!!! */
to allow better bundle chaining and/or little-overhead 
implicit bundle test.

Increasing priority (because of #18310)
Comment 3 Jesse Glick 2001-12-03 20:02:44 UTC
Makes sense.
Comment 4 Jesse Glick 2001-12-20 11:19:50 UTC
Oops, wrong dependency #.
Comment 6 Petr Nejedly 2002-01-29 17:10:20 UTC
We may also add interning of used strings (at least for keys)
for better memory performance - the keys will probably exist
in the form of string constants (i.e. interned strings)
in already loaded code (the code which asked for the bundle).
Comment 7 _ ttran 2002-01-30 09:40:59 UTC
String.intern() takes time.  We must balance it against memory
consumtion.  Do we know how slow String.intern() is?  For the first
occurrence of the string?  For subsequent occurrences?
Comment 8 Petr Nejedly 2002-01-30 11:19:35 UTC
Of course.
I've done some tests and interning both key and value have slown
it down by about 20% (20KB props file in about 30ms average).
No problem to write a platform test for it.
Comment 9 Jesse Glick 2002-01-31 13:55:51 UTC
For complaint #4, this will be obsolete when JDK bug #4405807 is
fixed, according to the note on the JDC:

Targeted for Hopper.

However the response to #4286358 is still discouraging.
Comment 10 Petr Nejedly 2002-02-01 14:28:59 UTC
The benchmark for interning keys and values is in
performance/test/perf/src/org/netbeans/performance/bundle/PropertiesTest.java

You can run it by
cd performance/test/perf
ant test -Dtests.specs=org.netbeans.performance.bundle.PropertiesTest
-Dtests.output=report/properties.xml

According it, the difference is minimal
Comment 11 Rochelle Raccah 2002-04-15 21:17:53 UTC
Until the replacement occurs, here are some ideas which might help
while still using ResourceBundle:
1) Make the 3 variables in the MergedBundle inner class final
2) Since branding assumes that the main bundle contains most of the
keys and the branded bundle only contains a subset, implement the
handleGetObject get method as follows (pseudo code):
if <sub1.contains key>
	return sub1.getObject (key);
return sub2.getObject (key);

See further discussion on the nbdev thread
<http://www.netbeans.org/servlets/BrowseList?listName=nbdev&by=thread&from=11266>
Comment 12 Jesse Glick 2002-04-15 21:53:45 UTC
Re. #1: probably won't make any difference under HotSpot, the class is
private anyway.

Re. #2: impossible due to Java #4286358.
Comment 13 Marek Grummich 2002-07-22 11:22:59 UTC
Set target milestone to TBD
Comment 14 Marek Grummich 2002-07-22 11:25:07 UTC
Set target milestone to TBD
Comment 15 Jesse Glick 2003-02-04 02:12:13 UTC
Created attachment 8783 [details]
Useless patch using binary ".cproperties" format
Comment 16 Jesse Glick 2003-02-04 02:13:25 UTC
Attached patch uses an unpublished binary (compiled) format for
.properties files; actually makes startup slightly slower, so forget
about it.
Comment 17 Jesse Glick 2003-02-04 04:01:19 UTC
Created attachment 8784 [details]
More successful patch
Comment 18 Jesse Glick 2003-02-04 04:12:37 UTC
This second patch does not use .cproperties. Instead it just changes
NbBundle to not use ResourceBundle.getBundle; it does its own
streamlined lookup:

- all locale + branding variants are searched together, rather than
having RB search locale variants and then add branding variants separately

- no MissingResourceException's inside the loop; would be easy to
provide a getBundleOrNull if desired

- looks for .properties before .class, since this is much more common
for NB code

- .class lookup checks for class file before trying Class.forName,
avoiding CNFE on a bundle miss

- bundles held with "timed soft cache": each item is held strongly for
30 sec since creation or last access, after which it is eligible for
soft collection (unless it is accessed again first)

- class loaders held weakly, as they should be

- no "backup" search for e.g. en_US if primary locale fails

- would be easy to provide a fast hasKey method if required for other
purposes

Results w/ JDK 1.4.1_01, sufficient RAM, Linux,
stable-with-apisupport, -nosplash -nogui:

Average times: unoptimized  10.0946069399516
                 std. dev.  0.0380714867971445
                 optimized  10.0149056037267
                 std. dev.  0.0173294426732414
Improvement:                0.0797013362248737
Percentage:                 0.789543730617569%

Modest but OK.

Next, will try on e.g. Japanese S1SEE to see how well it holds up with
combos of locales and branding relative to the older code.

Petr - I would like some opinion on whether the timed soft cache I
mentioned makes sense. It uses RequestProcessor to handle the timeout.
Might be a good idea to run some heap tests. I did run for a while
with println's to watch it in action, and it seemed reasonable enough;
bundles were only collected after a real GC cycle after they had not
been in use for a while.
Comment 19 Jesse Glick 2003-02-04 19:42:21 UTC
Results with S1SEE ja:JP are better:

Average times: unoptimized  14.0076841433843
                 std. dev.  0.0484515138844475
                 optimized  13.4769091447194
                 std. dev.  0.0213883329422276
Improvement:                0.530774998664857
Percentage:                 3.78917023850468%
Comment 20 Jesse Glick 2003-02-05 17:19:22 UTC
committed   * Up-To-Date  1.50       
openide/src/org/openide/util/NbBundle.java
added       * Up-To-Date  1.1        
openide/src/org/openide/util/TimedSoftReference.java
Comment 21 Petr Nejedly 2003-02-13 09:38:13 UTC
I've looked at the TimedSoftRef. Functionally is it OK,
but I'd suggest using a bit smarter scheme for timeout impl:
The (re)schedule call is relatively expensive, it does some
synchronization, allocates some objects and, worst of all,
each schedule(time!=0) leaves a trail in the TimerTask, a runnable
that will be executed (and possibly do nothing).
So the current impl would behave poorly for sequences of get().

I suggest using this approach:
On first get() schedule a task (now+30000) and record the
time-to-clear (now+30000). For subsequent get()s (TTC is nonzero),
just update the TTC to now+30000.
In the timeout handler, compare "now" to TTC and if it is bigger,
clear the reference and clear the TTC, otherwise reschedule the task
using schedule(TTC-now).
That means at most one reschedule each 30000ms per reference.

Maybe it can be implemented directly in the RPs reschedule,
but this way it it quite isolated and simple code.