Re: RFR: JDK-8042589: String.toLowerCase do not work for some concatenated strings
There is again another little optimization possible: instead hasSurr = true; we could use first = ~first; and save variable hasSurr. If there is register pressure (especially on older CPUs), this might be a performance advantage. Also I think we should not grow the result array for any character where mapLen srcCount, we could wait until the result array is actually full. -Ulf Am 17.07.2014 00:55, schrieb Xueming Shen: Still need a reviewer. On 07/09/2014 01:04 PM, Xueming Shen wrote: Hi, Please help review the change for JDK-8042589. Issue:https://bugs.openjdk.java.net/browse/JDK-8042589 webrev: http://cr.openjdk.java.net/~sherman/8042589/webrev/ This is a regression caused by the following change for #JDK-8032012, issue:https://bugs.openjdk.java.net/browse/JDK-8032012 webrev: http://cr.openjdk.java.net/~sherman/8032012/ discussion: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/024862.html It appears the last optimization for the surrogates we pushed in is incomplete. We still need to check isSurrogate() in the optimized non-surrogate loop, as the first (checked at the very beginning) might be triggered by a non-surrogate-upper/lowercase char. Thanks! -Sherman
Re: RFR: JDK-8042589: String.toLowerCase do not work for some concatenated strings
Additionally I think, instead of retrieving String lang = locale.getLanguage() multiple times in chain, we could pass lang to the methods instead locale. -Ulf Am 17.07.2014 11:54, schrieb Ulf Zibis: There is again another little optimization possible: instead hasSurr = true; we could use first = ~first; and save variable hasSurr. If there is register pressure (especially on older CPUs), this might be a performance advantage. Also I think we should not grow the result array for any character where mapLen srcCount, we could wait until the result array is actually full. -Ulf Am 17.07.2014 00:55, schrieb Xueming Shen: Still need a reviewer. On 07/09/2014 01:04 PM, Xueming Shen wrote: Hi, Please help review the change for JDK-8042589. Issue:https://bugs.openjdk.java.net/browse/JDK-8042589 webrev: http://cr.openjdk.java.net/~sherman/8042589/webrev/ This is a regression caused by the following change for #JDK-8032012, issue:https://bugs.openjdk.java.net/browse/JDK-8032012 webrev: http://cr.openjdk.java.net/~sherman/8032012/ discussion: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/024862.html It appears the last optimization for the surrogates we pushed in is incomplete. We still need to check isSurrogate() in the optimized non-surrogate loop, as the first (checked at the very beginning) might be triggered by a non-surrogate-upper/lowercase char. Thanks! -Sherman
Re: RFR: JDK-8042589: String.toLowerCase do not work for some concatenated strings
Let's fix the regression first, before make any more optimization :-) -Sherman On 07/17/2014 02:54 AM, Ulf Zibis wrote: There is again another little optimization possible: instead hasSurr = true; we could use first = ~first; and save variable hasSurr. If there is register pressure (especially on older CPUs), this might be a performance advantage. Also I think we should not grow the result array for any character where mapLen srcCount, we could wait until the result array is actually full. -Ulf Am 17.07.2014 00:55, schrieb Xueming Shen: Still need a reviewer. On 07/09/2014 01:04 PM, Xueming Shen wrote: Hi, Please help review the change for JDK-8042589. Issue:https://bugs.openjdk.java.net/browse/JDK-8042589 webrev: http://cr.openjdk.java.net/~sherman/8042589/webrev/ This is a regression caused by the following change for #JDK-8032012, issue:https://bugs.openjdk.java.net/browse/JDK-8032012 webrev: http://cr.openjdk.java.net/~sherman/8032012/ discussion: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/024862.html It appears the last optimization for the surrogates we pushed in is incomplete. We still need to check isSurrogate() in the optimized non-surrogate loop, as the first (checked at the very beginning) might be triggered by a non-surrogate-upper/lowercase char. Thanks! -Sherman
Re: RFR: JDK-8042589: String.toLowerCase do not work for some concatenated strings
Still need a reviewer. On 07/09/2014 01:04 PM, Xueming Shen wrote: Hi, Please help review the change for JDK-8042589. Issue:https://bugs.openjdk.java.net/browse/JDK-8042589 webrev: http://cr.openjdk.java.net/~sherman/8042589/webrev/ This is a regression caused by the following change for #JDK-8032012, issue:https://bugs.openjdk.java.net/browse/JDK-8032012 webrev: http://cr.openjdk.java.net/~sherman/8032012/ discussion: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/024862.html It appears the last optimization for the surrogates we pushed in is incomplete. We still need to check isSurrogate() in the optimized non-surrogate loop, as the first (checked at the very beginning) might be triggered by a non-surrogate-upper/lowercase char. Thanks! -Sherman
Re: RFR: JDK-8042589: String.toLowerCase do not work for some concatenated strings
On 07/09/2014 01:04 PM, Xueming Shen wrote: Hi, Please help review the change for JDK-8042589. Issue:https://bugs.openjdk.java.net/browse/JDK-8042589 webrev: http://cr.openjdk.java.net/~sherman/8042589/webrev/ This is a regression caused by the following change for #JDK-8032012, issue:https://bugs.openjdk.java.net/browse/JDK-8032012 webrev: http://cr.openjdk.java.net/~sherman/8032012/ discussion: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/024862.html It appears the last optimization for the surrogates we pushed in is incomplete. We still need to check isSurrogate() in the optimized non-surrogate loop, as the first (checked at the very beginning) might be triggered by a non-surrogate-upper/lowercase char. The fix looks okay. It may be useful to verify with the submitter and no other issue found since the fix for JDK-8032012 wasn't entirely straight forward. Mandy
RFR: JDK-8042589: String.toLowerCase do not work for some concatenated strings
Hi, Please help review the change for JDK-8042589. Issue:https://bugs.openjdk.java.net/browse/JDK-8042589 webrev: http://cr.openjdk.java.net/~sherman/8042589/webrev/ This is a regression caused by the following change for #JDK-8032012, issue:https://bugs.openjdk.java.net/browse/JDK-8032012 webrev: http://cr.openjdk.java.net/~sherman/8032012/ discussion: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/024862.html It appears the last optimization for the surrogates we pushed in is incomplete. We still need to check isSurrogate() in the optimized non-surrogate loop, as the first (checked at the very beginning) might be triggered by a non-surrogate-upper/lowercase char. Thanks! -Sherman
