harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexei Fedotov" <alexei.fedo...@gmail.com>
Subject Re: [general][performance] Serialization performance optimization results
Date Fri, 30 May 2008 14:09:49 GMT
Hello Aleksey,
Thank you for a diligent cleanup of the object stream code. If you are
looking for a sort of review for four JIRA issues, please find my
comments below concerning HARMONY-5847 [1].

[1] https://issues.apache.org/jira/browse/HARMONY-5847


* This is not transparent why you refer to the next handle differently
in the newly developed code:
+        Integer newHandle = nextHandle();
+        int newHandle = nextHandle();

 * Why you call to ObjectStreamClass.lookupStreamClass(superclass)
from the outside of readObjectNoData() and use three parameters
instead of two?
-                    readObjectNoData(object, superclass);
+                    readObjectNoData(object, superclass,
ObjectStreamClass.lookupStreamClass(superclass));

* I like TODO comments in the patch. From the other side some comments
look pretty mysterious:
+                // TODO: Here is the opportunity for enhancement
+                //      We can implement it through fast-path, without
+                //		setting up the context with public API
Both "it" and "context" are not defined. Also tabs make the code look
strangely aligned.

 * Why you use an assignment instead of just returning
updateReference(object, unshared)?
+        int handle = updateReference(object, unshared);
          return handle;

* It seems that inlining updateReference(object, unshared) in all
three locations would result in more compact and readable code because
the calls are always preceded with if (unshared) {}. Also it concerns
me that the previous code does some job for both cases.
     /**
+     * Updates the references table with new handle
+     *
+     * @param obj
+     *            Non-null object being updated
+     * @param unshared
+     *            whether the handle is unshared
+     */
+    private int updateReference(Object object, boolean unshared) {
+        int handle = nextHandle();
+
+        if (!unshared) {
+            objectsWritten.put(object, handle);
+        }
+
+        return handle;
+    }

 * Why cannot you set up the following properties in the constructor
instead of checking arePropertiesResolved on each access?
+    /**
+     * Cached class properties
+     */
+    private transient boolean isSerializable;
+    private transient boolean isExternalizable;
+    private transient boolean isProxy;
+    private transient boolean isEnum;

 * I believe the dot should be placed at <code>java.langClass</code>.


On Fri, May 30, 2008 at 3:39 PM, Aleksey Shipilev
<aleksey.shipilev@gmail.com> wrote:
> Hi,
>
> Here is the scrub of the serialization performance improvements we
> have today. I had used SPECjvm2008:serial as reference benchmark,
> running it on 8-core Xeon (E5440/2.86Ghz/HTN) / 24 Gb DDR2-667 /
> Windows 2003 EE SP1. All measurements were done in 5 iterations, 240
> secs per one iteration, max result was used as score. Scores are
> ops/min, the more the better. All JVMs were run in "-server -Xmx512M
> -Xms512M" mode. Measurement deviations are within 3%.
>
> Baseline measurements:
>  Sun 1.6.0_05 -server     145.0
>  Harmony r641838 -server 29.4
>
> So, Harmony performance was only 20% of RI on serialization workload.
>
> The improvements:
> (JIRA#, Harmony score, boost relative to baseline, status relative to RI.)
>
> -- already in trunk -----
> HARMONY-5635,  33.1,  13%,  23%
> HARMONY-5634,  35.4,  20%,  24%
> HARMONY-5640,  36.2,  23%,  25%
> HARMONY-5633,  68.7,  134%,  47%
> HARMONY-5735,  69.7,  137%,  48%
> HARMONY-5722,  80.4,  174%,  55%
> HARMONY-5756,  83.2,  183%,  57%
> HARMONY-5770,  85.1,  190%,  59%
>
> -- ready for review and commit ------
> HARMONY-5761,  88.4,  201%,  61%
> HARMONY-5829,  95.7,  226%,  66%
> HARMONY-5847,  110.7,  277%,  76%
> HARMONY-5771,  136.5,  365%,  94%
>
> -- need debug and review (estimated boosts) -----
> HARMONY-5713,  150.2,  411%,  >>>104%<<<
>
> The dry results of these measurements are:
>  1. After the committing rest of the _ready_ issues we will be close
> to Sun's performance. Nathan is working on HARMONY-5829 and
> HARMONY-5847 now, but HARMONY-5761 (WeakHashMap improvements) and
> HARMONY-5771 (IdentityHashMap improvements) are still orphaned. Can
> someone take them?
>
>  2. After the completion of HARMONY-5713 we will beat the RI on
> serialization benchmark.
>
> Thanks,
> Aleksey.
>



-- 
With best regards,
Alexei

Mime
View raw message