db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Knut Anders Hatlen (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-2114) Let Clock embed a HashMap rather than inherit from Hashtable
Date Thu, 15 Mar 2007 10:05:10 GMT

    [ 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.


Mime
View raw message