Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Mon, 7 Mar 2022 13:05:09 GMT, Jim Laskey wrote: >> Would it be just as effective and improve performance more broadly to cache >> in DecimalFormatSymbols.getInstance()? >> >> Declarations should be private unless there is a package need. >> In this case, the only access to should be via the method. > > Will investigate Performance is on par when cached in DecimalFormatSymbols. Will switch to that direction. - PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Mon, 7 Mar 2022 08:25:19 GMT, Stephen Colebourne wrote: >> Jim Laskey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Too many 'the' > > Just to note that there is also some caching in `DecimalStyle`. It might be > possible to reuse `DecimalStyle` here (if it was extended to support grouping > separators). @jodastephen True that. It would also be nice if `DecimalFormatSymbols` also contained grouping size. Will investigate but will use a separate PR for the fallout. - PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 20:04:42 GMT, Roger Riggs wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Too many 'the'
>
> src/java.base/share/classes/java/util/Formatter.java line 2025:
>
>> 2023:
>> 2024: // Caching zero.
>> 2025: static char getZero(Locale locale) {
>
> Perhaps should be private.
> The comment says caching zero, but its really the DecimalFormatSymbols that
> are cached.
Noted.
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Mon, 7 Mar 2022 00:33:53 GMT, Naoto Sato wrote: >>> will now try and update/use this cached class level static state DFS. That >>> would thus require some kind of thread safety semantics to be implemented >>> for this new getDecimalFormatSymbols(Locale) method, isn't it? >> >> A bit more closer look at the code and it appears to me that the use of : >> >> DecimalFormatSymbols dfs = DFS; >> >> and then working off that local variable prevents any kind of race issues >> that might be caused due to multi-thread access. Of course it still means >> that multiple threads might still go ahead and do a: >> >> dfs = DecimalFormatSymbols.getInstance(locale); >> >> on first access (when `DFS` is null) but that in itself should be harmless. >> >> If this is intentional (which I suspect it is), should some comment be added >> in this method explaining this multi-thread access detail? > > Initially, I thought of the same thing (not the `Formatter` but > `DecimalFormatSymbols` itself, as it is also not thread safe), but I > concluded it was OK, as there is no mutation going on. I agree with Jaikiran > that some comments might help here. Noted - PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 19:02:29 GMT, Naoto Sato wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Too many 'the'
>
> src/java.base/share/classes/java/util/Formatter.java line 2026:
>
>> 2024: // Caching zero.
>> 2025: static char getZero(Locale locale) {
>> 2026: return locale == null ? '0' :
>> getDecimalFormatSymbols(locale).getZeroDigit();
>
> While we are at it, it would be beneficial to cache locale-dependent grouping
> and decimal separators too.
Noted
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 21:14:26 GMT, Roger Riggs wrote: >> As a separate/future issue, perhaps the constructors should be deprecated to >> nudge people to using the static `getInstance` methods. > > Would it be just as effective and improve performance more broadly to cache > in DecimalFormatSymbols.getInstance()? > > Declarations should be private unless there is a package need. > In this case, the only access to should be via the method. Will investigate - PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 21:17:50 GMT, Jim Laskey wrote:
>> Several attempts have been made to improve Formatter's numeric performance
>> by caching the current Locale zero. Such fixes, however, ignore the real
>> issue, which is the slowness of fetching DecimalFormatSymbols. By directly
>> caching DecimalFormatSymbols in the Formatter, this enhancement streamlines
>> the process of accessing Locale DecimalFormatSymbols and specifically
>> getZeroDigit(). The result is a general improvement in the performance of
>> numeric formatting.
>>
>>
>> @Benchmark
>> public void bigDecimalDefaultLocale() {
>> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalLocaleAlternate() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalThaiLocale() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void integerDefaultLocale() {
>> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerLocaleAlternate() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerThaiLocale() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>>
>> Before:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>>
>> After:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Too many 'the'
Just to note that there is also some caching in `DecimalStyle`. It might be
possible to reuse `DecimalStyle` here (if it was extended to support grouping
separators).
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Sun, 6 Mar 2022 15:00:47 GMT, Jaikiran Pai wrote:
>> src/java.base/share/classes/java/util/Formatter.java line 2012:
>>
>>> 2010: public final class Formatter implements Closeable, Flushable {
>>> 2011: // Caching DecimalFormatSymbols
>>> 2012: static DecimalFormatSymbols DFS = null;
>>
>> Hello Jim,
>> The javadoc of `Formatter` states:
>>
>>>
>>> Formatters are not necessarily safe for multithreaded access. Thread
>>> safety is optional and is the responsibility of users of methods in this
>>> class.
>>>
>>
>> So I would think that user applications would typically be synchronizing on
>> the instance of a `Formatter` for any multi-threaded use of a formatter
>> instance.
>>
>> If I'm reading this changed code correctly, even if user applications have
>> properly synchronized on a `Formatter` instance, it's now possible that 2
>> separate instances of a `Formatter` being concurrently accessed (i.e. in
>> different threads) will now try and update/use this cached class level
>> `static` state `DFS`. That would thus require some kind of thread safety
>> semantics to be implemented for this new `getDecimalFormatSymbols(Locale)`
>> method, isn't it?
>
>> will now try and update/use this cached class level static state DFS. That
>> would thus require some kind of thread safety semantics to be implemented
>> for this new getDecimalFormatSymbols(Locale) method, isn't it?
>
> A bit more closer look at the code and it appears to me that the use of :
>
> DecimalFormatSymbols dfs = DFS;
>
> and then working off that local variable prevents any kind of race issues
> that might be caused due to multi-thread access. Of course it still means
> that multiple threads might still go ahead and do a:
>
> dfs = DecimalFormatSymbols.getInstance(locale);
>
> on first access (when `DFS` is null) but that in itself should be harmless.
>
> If this is intentional (which I suspect it is), should some comment be added
> in this method explaining this multi-thread access detail?
Initially, I thought of the same thing (not the `Formatter` but
`DecimalFormatSymbols` itself, as it is also not thread safe), but I concluded
it was OK, as there is no mutation going on. I agree with Jaikiran that some
comments might help here.
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Sat, 5 Mar 2022 14:20:40 GMT, Jaikiran Pai wrote: > will now try and update/use this cached class level static state DFS. That > would thus require some kind of thread safety semantics to be implemented for > this new getDecimalFormatSymbols(Locale) method, isn't it? A bit more closer look at the code and it appears to me that the use of : DecimalFormatSymbols dfs = DFS; and then working off that local variable prevents any kind of race issues that might be caused due to multi-thread access. Of course it still means that multiple threads might still go ahead and do a: dfs = DecimalFormatSymbols.getInstance(locale); on first access (when `DFS` is null) but that in itself should be harmless. If this is intentional (which I suspect it is), should some comment be added in this method explaining this multi-thread access detail? - PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 21:17:50 GMT, Jim Laskey wrote:
>> Several attempts have been made to improve Formatter's numeric performance
>> by caching the current Locale zero. Such fixes, however, ignore the real
>> issue, which is the slowness of fetching DecimalFormatSymbols. By directly
>> caching DecimalFormatSymbols in the Formatter, this enhancement streamlines
>> the process of accessing Locale DecimalFormatSymbols and specifically
>> getZeroDigit(). The result is a general improvement in the performance of
>> numeric formatting.
>>
>>
>> @Benchmark
>> public void bigDecimalDefaultLocale() {
>> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalLocaleAlternate() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalThaiLocale() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void integerDefaultLocale() {
>> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerLocaleAlternate() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerThaiLocale() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>>
>> Before:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>>
>> After:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Too many 'the'
src/java.base/share/classes/java/util/Formatter.java line 2012:
> 2010: public final class Formatter implements Closeable, Flushable {
> 2011: // Caching DecimalFormatSymbols
> 2012: static DecimalFormatSymbols DFS = null;
Hello Jim,
The javadoc of `Formatter` states:
>
> Formatters are not necessarily safe for multithreaded access. Thread safety
> is optional and is the responsibility of users of methods in this class.
>
So I would think that user applications would typically be synchronizing on the
instance of a `Formatter` for any multi-threaded use of a formatter instance.
If I'm reading this changed code correctly, it's now possible that 2 separate
instances of a `Formatter` being concurrently accessed (i.e. in different
threads) will now try and update/use this cached class level `static` state
`DFS`. That would thus require some kind of thread safety semantics to be
implemented for this new `getDecimalFormatSymbols(Locale)` method, isn't it?
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 21:17:50 GMT, Jim Laskey wrote:
>> Several attempts have been made to improve Formatter's numeric performance
>> by caching the current Locale zero. Such fixes, however, ignore the real
>> issue, which is the slowness of fetching DecimalFormatSymbols. By directly
>> caching DecimalFormatSymbols in the Formatter, this enhancement streamlines
>> the process of accessing Locale DecimalFormatSymbols and specifically
>> getZeroDigit(). The result is a general improvement in the performance of
>> numeric formatting.
>>
>>
>> @Benchmark
>> public void bigDecimalDefaultLocale() {
>> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalLocaleAlternate() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalThaiLocale() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void integerDefaultLocale() {
>> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerLocaleAlternate() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerThaiLocale() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>>
>> Before:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>>
>> After:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Too many 'the'
src/java.base/share/classes/java/text/DecimalFormatSymbols.java line 192:
> 190:
> 191: /**
> 192: * Gets the locale used to create this instance.
Nit: Since `@return` has the same description, `{@return ...}` can be used here.
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 21:17:50 GMT, Jim Laskey wrote:
>> Several attempts have been made to improve Formatter's numeric performance
>> by caching the current Locale zero. Such fixes, however, ignore the real
>> issue, which is the slowness of fetching DecimalFormatSymbols. By directly
>> caching DecimalFormatSymbols in the Formatter, this enhancement streamlines
>> the process of accessing Locale DecimalFormatSymbols and specifically
>> getZeroDigit(). The result is a general improvement in the performance of
>> numeric formatting.
>>
>>
>> @Benchmark
>> public void bigDecimalDefaultLocale() {
>> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalLocaleAlternate() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalThaiLocale() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void integerDefaultLocale() {
>> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerLocaleAlternate() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerThaiLocale() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>>
>> Before:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>>
>> After:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Too many 'the'
src/java.base/share/classes/java/util/Formatter.java line 2025:
> 2023:
> 2024: // Caching zero.
> 2025: static char getZero(Locale locale) {
Perhaps should be private.
The comment says caching zero, but its really the DecimalFormatSymbols that are
cached.
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 20:00:54 GMT, Roger Riggs wrote:
>> I am afraid people are still using constructors for creating a locale,
>> instead of the factory method that was added later. Since `new Locale("en")
>> == new Locale("en")` returns `false`, I'd still expect `equals()` to compare
>> locales. As to the constants, the number of them is relatively small, IMO.
>
> As a separate/future issue, perhaps the constructors should be deprecated to
> nudge people to using the static `getInstance` methods.
Would it be just as effective and improve performance more broadly to cache in
DecimalFormatSymbols.getInstance()?
Declarations should be private unless there is a package need.
In this case, the only access to should be via the method.
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
> Several attempts have been made to improve Formatter's numeric performance by
> caching the current Locale zero. Such fixes, however, ignore the real issue,
> which is the slowness of fetching DecimalFormatSymbols. By directly caching
> DecimalFormatSymbols in the Formatter, this enhancement streamlines the
> process of accessing Locale DecimalFormatSymbols and specifically
> getZeroDigit(). The result is a general improvement in the performance of
> numeric formatting.
>
>
> @Benchmark
> public void bigDecimalDefaultLocale() {
> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalLocaleAlternate() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalThaiLocale() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void integerDefaultLocale() {
> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d", x);
> }
>
> @Benchmark
> public void integerLocaleAlternate() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
> @Benchmark
> public void integerThaiLocale() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
>
> Before:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>
> After:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
Jim Laskey has updated the pull request incrementally with one additional
commit since the last revision:
Too many 'the'
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/7703/files
- new: https://git.openjdk.java.net/jdk/pull/7703/files/50944ba0..9ac0c283
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=7703=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk=7703=01-02
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/7703.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7703/head:pull/7703
PR: https://git.openjdk.java.net/jdk/pull/7703
