DataNucleus

StringBuffer used throughout, when StringBuilder should be preferred
--------------------------------------------------------------------

                 Key: NUCCORE-555
                 URL: http://www.datanucleus.org/servlet/jira/browse/NUCCORE-555
             Project: DataNucleus Core
          Issue Type: Bug
            Reporter: Fernando Padilla
            Priority: Minor


Actually, if you search for "StringBuffer" you find at least 548 instances of it, and even a cursory review, it looks like most of those situations they should be using StringBuilder instead since they do not need the be synchronized.  This will help with performance.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.datanucleus.org/servlet/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

Concurrency Bug: unsynchronized WeakHashMap in OIDFactory
---------------------------------------------------------

                 Key: NUCCORE-553
                 URL: http://www.datanucleus.org/servlet/jira/browse/NUCCORE-553
             Project: DataNucleus Core
          Issue Type: Bug
            Reporter: Fernando Padilla
            Priority: Blocker


Our systems keep hanging, and we have lots of threads stuck on this stack trace:

   java.lang.Thread.State: RUNNABLE
        at java.util.WeakHashMap.get(WeakHashMap.java:355)
        at org.datanucleus.identity.OIDFactory.getInstance(OIDFactory.java:54)
        at org.datanucleus.store.mapped.mapping.OIDMapping.getObject(OIDMapping.java:125)
        at org.datanucleus.store.rdbms.query.PersistentClassROF.getObject(PersistentClassROF.java:360)
        at org.datanucleus.store.rdbms.query.ResultClassROF.getObject(ResultClassROF.java:272)
        at org.datanucleus.store.rdbms.query.ForwardQueryResult.nextResultSetElement(ForwardQueryResult.java:135)
        at org.datanucleus.store.rdbms.query.ForwardQueryResult$QueryResultIterator.next(ForwardQueryResult.java:302)
...
..
.


I did a yahoo search and found the mention below, that states that they ran into it while using WeakHashMap while not properly synchronized.  And this is the case here.

http://jira.codehaus.org/browse/XSTR-584


So we simply need to add synchronization around the WeakHashMap access.


ps - I actually wonder, if using a WeakHashMap to cache OIDs is actually beneficial.  Just a hypothesis, that we could/should test.  With the latest GCs it's sometimes better to simply create short lived objects, rather than try managing pools.  Also, I noticed that to generate the Key for the cache, pretty much does the same work (concat class/oid) as what the constructor for OIDImpl (and OIDImplKodo that we use) does.  So it doesn't seem to save any work what so ever.  Just add another single point of synchronization and bottleneck...  I propose to simply get rid of the caching. :)

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.datanucleus.org/servlet/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

OIDImpl, OIDImplKodo use StringBuffer within constructor, instead of StringBuilder
----------------------------------------------------------------------------------

                 Key: NUCCORE-554
                 URL: http://www.datanucleus.org/servlet/jira/browse/NUCCORE-554
             Project: DataNucleus Core
          Issue Type: Bug
            Reporter: Fernando Padilla
            Priority: Minor


As you know, the difference between StringBuffer and StringBuilder is that StringBuffer is implicitly synchronized.  But within the OIDImpl constructor, it's not multithreaded, so it's just wasted overhead.

BEFORE:
        StringBuffer s = new StringBuffer();
        s.append(this.pcClass);
        s.append(oidSeparator);
        s.append(this.oid.toString());

AFTER:
StringBuilder s = new StringBuilder();
...


----

And actually, for pre-determined appends like this, using a StringBuffer/Builder doesn't really make a difference from the "+" operator, so you can make the code more legible by simply concatenating the values directly: 

String s = this.pcClass + oidSeparator + this.oid;


-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://www.datanucleus.org/servlet/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira