Community
Participate
Working Groups
Production stack dumps show following code to be the most significant bottleneck in TopLink and for our application scalability: WebContainer : 1" daemon prio=10 tid=0x000000010438e1a0 nid=0x8b runnable [0xfffffffd3f5f8000..0xfffffffd3f5ff8a8] at java.lang.Class.isPrimitive(Native Method) at oracle.toplink.internal.helper.ConversionManager.convertObject(ConversionManager.java:83) at oracle.toplink.internal.databaseaccess.DatasourcePlatform.convertObject(DatasourcePlatform.java:142) at oracle.toplink.oraclespecific.Oracle9Platform.convertObject(Oracle9Platform.java:286) Study of usage of the code show that the if statement doing this check is very rarely or ever true, which mean it should be down in the method to end-up be rarely called. The statement to move down the method is: // Check if object is instance of the real class for the primitive class. if (javaClass.isPrimitive() && (((sourceObject instanceof Boolean) && (javaClass == ClassConstants.PBOOLEAN)) || ((sourceObject instanceof Long) && (javaClass == ClassConstants.PLONG)) || ((sourceObject instanceof Integer) && (javaClass == ClassConstants.PINT)) || ((sourceObject instanceof Float) && (javaClass == ClassConstants.PFLOAT)) || ((sourceObject instanceof Double) && (javaClass == ClassConstants.PDOUBLE)) || ((sourceObject instanceof Byte) && (javaClass == ClassConstants.PBYTE)) || ((sourceObject instanceof Character) && (javaClass == ClassConstants.PCHAR)) || ((sourceObject instanceof Short) && (javaClass == ClassConstants.PSHORT)))) { return sourceObject; } An other optimization is that it's obviously cheaper to compare two instance than doing an instanceof, so the compare above should start with the instance equality. References: - MetaLink SR 7493211.992 - MetaLink Bug 8394827
Created attachment 133617 [details] The full fix handling the performance issue.
It is doubtful that this code is the most significant bottleneck in your application however the patch does have some merit as a micro optimization. This block of code should not be moved it is expected that primitive types will be the most popular data type in a general application. However the isPrimitive check should be removed as proposed and the instanceof checks which are redundant should be removed as well. A simple benchmark will show that the == comparison for all of the primitive classes will be considerably faster than the is primitive check.
I think Gordon expectation that the method is often called with native type is false. I have put a breakpoint on return of the if statement and it has never been hit after I triggered all the popular screen of my complex production application. You can verify our statement by looing at our production stack dumps we have attached to the MetaLink SR and bug. We are not making this up. The most important fix it to move down the if statement like the patch we have provided. Please put a breakpoint like we did on the return statement and one on the more important statement and run your test suite, you will see easily what is the most important optimization.
Fixed as suggested.
Created attachment 133951 [details] Replacement Fix, faster and SDO tests are now passing
Recent patch is still not applied, and older fix was rollback so reopen bug.
I am looking at the fix and attempting to run the tests on it in trunk.
I have checked in a fix based on the fix provided here. I applied the fix in whole except for the reordering of the LONG/STRING operations. This part of the fix would likely have a (very minor) but potential negative performance impact on models with lots of strings. There may be a case for this change, but I would need to see a profile to check in a change based on it.
Backported change to 1.1.2.
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink
