Verified at r732988. Thanks so much, Tim! 2009/1/9 Tim Ellison <[email protected]>
> Jim Yu wrote: > > Hi Aleksey, > > The final patch is ready now. Please help to review : ) > > Patch V1 (as reviewed) applied at r732988. > > Regards, > Tim > > > 2009/1/9 Aleksey Shipilev <[email protected]> > > > >> Jim, > >> > >> Given the optimal version is based on your patch, can you produce the > >> final patch? > >> > >> IMO, it shouldn't include in-place conversion, rather it should be > >> pure Integer.toString() optimization for now. > >> > >> Thanks, > >> Aleksey. > >> > >> On Thu, Jan 8, 2009 at 5:45 PM, Jim Yu <[email protected]> wrote: > >>> Hi Aleksey and Kevin, > >>> It is a good news! The "jim-aleksey" patch is very awesome! I agree to > >> apply > >>> this patch. > >>> > >>> 2009/1/8 Aleksey Shipilev <[email protected]> > >>> > >>>> Hi Jim, Kevin, > >>>> > >>>> There are good things in both patches: Jim's one does the tricky > >>>> lookup for big integers, which benefit for them; Kevin's one does > >>>> division very cheaply. > >>>> > >>>> I had tried several implementations of radix-10 Integer.toString(): > >>>> - (original) baseline, present in trunk > >>>> - (kevin) Kevin's original one from HARMONY-6056 > >>>> - (kevin-aleksey) Kevin's one + guessing the buffer size > >>>> - (jim) Jim's one from HARMONY-6068 > >>>> - (jim-aleksey) Jim's one + optimized division for small integers > >>>> > >>>> The merged patch is attached to > >>>> https://issues.apache.org/jira/browse/HARMONY-6068. > >>>> > >>>> The results are below. In short, Jim's patch rocks! :) It rocks even > >>>> more on small integers with Kevin's ideas. The best one is > >>>> "jim-aleksey", but it's too heavy to just copy-paste to String(String, > >>>> int) constructor, so there's a need in in-place Integer.toString() > >>>> conversion. > >>>> > >>>> What do you think? > >>>> > >>>> -------- > >>>> (ops/msec, the more the better) > >>>> > >>>> original-1024Mb: > >>>> *3969, 2612, 1944, 1481, 1168, > >>>> 3831, 2557, 1906, 1447, 1153, > >>>> 3692, 2635, 1971, 1495, 1186, > >>>> 3914, 2626, 1958, 1482, 1170, > >>>> 3947, 2630, 1961, 1478, 1179, > >>>> > >>>> kevin-1024Mb: > >>>> 5041, 3343, 2484, 1883, 1566, > >>>> 5010, 3329, 2454, 1907, 1543, > >>>> 5009, 3325, 2473, 1886, 1539, > >>>> 4969, 3314, 2450, 1884, 1549, > >>>> 4901, 3271, 2457, 1868, 1542, > >>>> > >>>> kevin-aleksey-1024Mb: > >>>> 5041, 4043, 3364, 2867, 2340, > >>>> 5010, 3962, 3368, 2852, 2333, > >>>> 4993, 3992, 3318, 2857, 2303, > >>>> 4953, 3997, 3318, 2847, 2313, > >>>> 4946, 3947, 3311, 2841, 2303, > >>>> > >>>> jim-1024Mb: > >>>> 4681, 3566, 3019, 2976, 2897, > >>>> 4810, 3558, 3054, 2954, 2880, > >>>> 4802, 3530, 2982, 2943, 2868, > >>>> 4795, 3481, 2991, 2918, 2862, > >>>> 4744, 3476, 2954, 2902, 2849, > >>>> > >>>> jim-aleksey-1024Mb: > >>>> 5041, 3977, 2919, 2964, 2860, > >>>> 5066, 3918, 2951, 2929, 2873, > >>>> 5090, 3918, 2886, 2920, 2875, > >>>> 5018, 3918, 2938, 2934, 2878, > >>>> 4978, 3917, 2921, 2868, 2831, > >>>> > >>>> > >>>> Thanks, > >>>> Aleksey. > >>>> > >>>> On Thu, Jan 8, 2009 at 1:11 PM, Jim Yu <[email protected]> wrote: > >>>>> Thanks Aleksey! I think my patch is a general solution for this case > >> and > >>>> has > >>>>> no dependency on any optimized feature of J9 VM : ) > >>>>> 2009/1/8 Aleksey Shipilev <[email protected]> > >>>>> > >>>>>> That's awesome, Jim! > >>>>>> Thanks for coming in :) > >>>>>> > >>>>>> However, you seem to be missing the point. I was talking about > >>>>>> refactoring Kevin's patch to fit the stated requirements. This is > >>>>>> merely because Kevin claim the patch already gives the boost for > >>>>>> SPECjbb2005. Kevin already has the specialized conversion in > >>>>>> String(String, int) private constructor. > >>>>>> > >>>>>> Nevertheless, your specialized version looks good with regards to > >>>>>> performance numbers, we need to test what's better for radix-10 > >>>>>> conversion: Jim's version or Kevin's one now. > >>>>>> > >>>>>> Thanks a lot, > >>>>>> Aleksey. > >>>>>> > >>>>>> On Thu, Jan 8, 2009 at 12:11 PM, Jim Yu <[email protected]> > >> wrote: > >>>>>>> Hi Aleksey, > >>>>>>> I very agree with you and I have implemented an optimized algorithm > >>>> for > >>>>>>> Integer.toString(int) method. Thanks to your benchmark, here are > >> the > >>>> test > >>>>>>> results[1] on my windows platform. I've raised a JIRA at > >>>>>>> https://issues.apache.org/jira/browse/HARMONY-6068 > >>>>>>> [1] > >>>>>>> Result for Harmony java6 branch: > >>>>>>> (String)base + (int)add: > >>>>>>> ------------------------------------------- > >>>>>>> base length (vars with rows): 0..2..10 > >>>>>>> add length (vars with cols): 0..2..10 > >>>>>>> > >>>>>>> loop duration = 100 msecs > >>>>>>> target variance = 0.05 > >>>>>>> > >>>>>>> ops/msec, the more the better: > >>>>>>> 6721, 6096, *4650, *3846, *3178, > >>>>>>> *8080, *5833, *4447, 3731, 3048, > >>>>>>> *7985, *5848, 4788, 3727, *3114, > >>>>>>> *7891, 5592, *4389, *3560, 3048, > >>>>>>> 8388, 5607, *4522, 3727, 3051, > >>>>>>> > >>>>>>> After applied my patch: > >>>>>>> (String)base + (int)add: > >>>>>>> ------------------------------------------- > >>>>>>> base length (vars with rows): 0..2..10 > >>>>>>> add length (vars with cols): 0..2..10 > >>>>>>> > >>>>>>> loop duration = 100 msecs > >>>>>>> target variance = 0.05 > >>>>>>> > >>>>>>> ops/msec, the more the better: > >>>>>>> 8322, 6721, 4791, 4788, 4788, > >>>>>>> 8388, 6721, 5156, *5012, 4797, > >>>>>>> 8388, 6707, 5161, *4963, 4795, > >>>>>>> 8388, 6707, *5126, 4802, 4788, > >>>>>>> *8048, 6700, *5021, 4802, *4687, > >>>>>>> > >>>>>>> 2009/1/7 Aleksey Shipilev <[email protected]> > >>>>>>> > >>>>>>>> Ok, we can implement the in-place Integer.toString() and > >> specialize > >>>>>>>> the radix-10 conversion in Integer. Then Classlib performance guys > >>>>>>>> might use the inplace conversion to optimize StringBuilder > >>>> performance > >>>>>>>> or even catch the concatenation like J9 does. > >>>>>>>> > >>>>>>>> My idea is to share whatever optimization between all VMs that use > >>>> the > >>>>>>>> Classlib. > >>>>>>>> > >>>>>>>> Thanks, > >>>>>>>> Aleksey. > >>>>>>>> > >>>>>>>> On Wed, Jan 7, 2009 at 5:46 PM, Tim Ellison < > >> [email protected]> > >>>>>> wrote: > >>>>>>>>> Aleksey Shipilev wrote: > >>>>>>>>>> Am I understanding right that private String(String, int) is > >>>> inlined > >>>>>>>>>> by J9 JIT when (String)s1 + (int)v1 is required? > >>>>>>>>> Yes - so for DRLVM, Kevin's patch in HARMONY-6056 will be > >> impotent. > >>>>>>>>> Regards, > >>>>>>>>> Tim > >>>>>>>>> > >>>>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> -- > >>>>>>> Best Regards, > >>>>>>> Jim, Jun Jie Yu > >>>>>>> > >>>>>>> China Software Development Lab, IBM > >>>>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> Best Regards, > >>>>> Jim, Jun Jie Yu > >>>>> > >>>>> China Software Development Lab, IBM > >>>>> > >>> > >>> > >>> -- > >>> Best Regards, > >>> Jim, Jun Jie Yu > >>> > >>> China Software Development Lab, IBM > >>> > > > > > > > -- Best Regards, Jim, Jun Jie Yu China Software Development Lab, IBM
