Sherman, thanks for your review ...
Am 07.10.2009 19:56, Xueming Shen schrieb:
Though I did not write the cache code my reading suggests the existing
cache
impl tries to avoid the relatively "expensive" synchronization for
most use scenarios,
your approach however hits the synchronization for each/every lookup
invocation.
So you should at least consider to put the sync block in a separate
lookup2.
Agreed:
(But why separate lookup2? I suspect that Charset.forName() would occur
in excessive loops, so hotspot potentially cold inline it.)
private static Charset lookup(String charsetName) {
if (charsetName == null)
throw new IllegalArgumentException("Null charset name");
CacheItem ci = cache[0];
if (ci != null && charsetName.equals(ci.name))
return ci.charset;
synchronized (Charset.class) {
int pos;
for (pos=1; ; pos++) {
...
}
}
Is synchronization more expensive, than instantiating new Object[] each
time + 2 castings (to String + to Charset)?
But personally I doubt we really care this "race condition", it's a
cache, a cache miss
is not a disaster,
OK, but why didn't you you reject Bug Id 6881442 with the same
argumentation.
... and missing the class's name in Class.name could only happen once,
in contrast to "my" race condition, which theoretically could happen
every time.
in fact in most cases, since the charset has been in the cache
already, it should be in the providers' "cache" as well, especially
for the bundled
standard & extended charset provider, the lookup actually is 1 or 2
O(1) hashmap
lookup
... it's 2 hashmap lookup, see
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6850361
+ 2 hashmap lookup for extended charset + 2n cost if in supplemental
provider
(yes, an additional name canonicalization and the "expensive" sync).
+ check extendedProviderProbed + synchronize latter + resolve SoftReference
(in fail: + hashmap lookup for class name + reflective instantiation +
caching)
So maybe we can do something like
b = cache1;
cache1 = a;
cache2 = b;
Hm, if content of cache1 was updated to content, which is hold in a by
other thread, saving cache1in b doesn't help.
(Maybe I didn't understand your trick.)
to make the situation a little better, or maybe worse:-)
And what do you think about breaking the endless loop, while getting the
default charset, and what about avoiding to cache wrong default charset?
-Ulf
