[
https://issues.apache.org/jira/browse/DERBY-2114?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12481098
]
Knut Anders Hatlen commented on DERBY-2114:
-------------------------------------------
The patch generally looks good. I verified that the unsynchronized HashMap
methods were called either from the inside of a block synchronized on the Clock
object or from a method whose javadoc said the caller should be synchronized on
Clock. The only exception was the new Clock.values() method, which requires the
caller to be synchronized but doesn't say so in its javadoc. Perhaps you could
add a comment about it in the javadoc?
I was wondering whether it would be better if the new values() method returned
a copy of the values in the cache. The reason is this code in
diag.StatementCache's constructor:
+ synchronized(lcc.statementCache) {
+ final Collection values =
lcc.statementCache.values();
+ data = new Vector(values.size());
+ for (Iterator i = values.iterator();
i.hasNext(); ) {
+ CachedItem ci = (CachedItem) i.next();
+ CachedStatement cs = (CachedStatement)
ci.getEntry();
+ GenericPreparedStatement ps =
(GenericPreparedStatement)
+ cs.getPreparedStatement();
+ data.addElement(ps);
+ }
+ } // synchronized(lcc.statementCache)
By requiring the caller to synchronize on the Clock object, I feel that we are
exposing an implementation detail that could be hidden. If we return a copy,
the synchronization could be done inside values() instead. Since it is only
used in a diagnostic VTI, I don't think the extra cost of creating a shallow
copy should be a problem.
> Let Clock embed a HashMap rather than inherit from Hashtable
> ------------------------------------------------------------
>
> Key: DERBY-2114
> URL: https://issues.apache.org/jira/browse/DERBY-2114
> Project: Derby
> Issue Type: Improvement
> Components: Performance
> Affects Versions: 10.2.1.6
> Reporter: Dyre Tjeldvoll
> Assigned To: Dyre Tjeldvoll
> Priority: Trivial
> Fix For: 10.3.0.0
>
> Attachments: derby-2114.v1.diff, derby-2114.v1.stat
>
>
> Clock currently inherits from Hashtable, but the use of Hashtable is really
> an implementation detail that would benefit from being hidden as private
> member. All access to the hashtable happens inside sychronized blocks so it
> is safe to substitute a HashMap. This change appears to trigger a small
> increase in throughput, as measured by the average TPS number obtained by
> running the select client from DERBY-1961 repeatedly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
