On Fri, 8 May 2026 19:19:28 GMT, Justin Lu <[email protected]> wrote:
>> Daniel Gredler has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update three comments which mention vectors
>
> src/java.base/share/classes/java/text/AttributedString.java line 354:
>
>> 352: * empty Map.
>> 353: */
>> 354: public void addAttributes(Map<? extends Attribute, ?> attributes,
>
> Since we moved from `Vector` to `Map`, I think this is now susceptible to
> `ConcurrentModificationException` when called at the same time that the
> `AttributedCharacterIterator` is iterated. I don't think it matters though
> since this method isn't specified to be thread safe and
> `AttributedCharacterIterator` does not define a concurrent modification
> policy, so I think it's OK.
I also think it's OK, but it does raise an interesting question. Per the
comments at the top of `AttributedStringIterator`, these classes makes some
effort to synchronize access to the `AttributedString`. However, there is a gap
in this protection because 2 of the 3 mutating methods (both named
`addAttribute`) synchronize by delegating to the private synchronized
`addAttributeImpl` method, while the third method (`addAttributes`) does not go
through this private method, and is missing any synchronization. I think it
might be a good idea to open a separate bug to address this synchronization gap
in `addAttributes`. Thoughts?
> test/micro/org/openjdk/bench/java/text/AttributedStringBench.java line 29:
>
>> 27: import java.text.AttributedCharacterIterator;
>> 28: import java.text.AttributedString;
>> 29: import java.util.Map;
>
> Looks unused.
Thanks, removed.
> test/micro/org/openjdk/bench/java/text/AttributedStringBench.java line 72:
>
>> 70:
>> 71: @Benchmark
>> 72: public void iterationWithNoAttributes(Blackhole blackhole) {
>
> All the `iteration` tests are measuring missing key lookups, which is good to
> measure but is also worst-case. Might be worth adding present key lookups for
> transparency, so both cases are measured.
I've added two "key present" tests (`iteratePresentWithOneAttribute` and
`iteratePresentWithThreeAttributes`), but the performance improvement actually
looks larger than with missing keys.
Before PR changes:
Benchmark
Mode Cnt Score Error Units
AttributedStringBench.createWithNoAttributes
avgt 15 0.003 ± 0.001 us/op
AttributedStringBench.createWithNoAttributes:gc.alloc.rate
avgt 15 11529.849 ± 397.065 MB/sec
AttributedStringBench.createWithNoAttributes:gc.alloc.rate.norm
avgt 15 32.000 ± 0.001 B/op
AttributedStringBench.createWithNoAttributes:gc.count
avgt 15 224.000 counts
AttributedStringBench.createWithNoAttributes:gc.time
avgt 15 185.000 ms
AttributedStringBench.createWithOneAttribute
avgt 15 0.045 ± 0.001 us/op
AttributedStringBench.createWithOneAttribute:gc.alloc.rate
avgt 15 7993.680 ± 232.021 MB/sec
AttributedStringBench.createWithOneAttribute:gc.alloc.rate.norm
avgt 15 376.000 ± 0.001 B/op
AttributedStringBench.createWithOneAttribute:gc.count
avgt 15 157.000 counts
AttributedStringBench.createWithOneAttribute:gc.time
avgt 15 133.000 ms
AttributedStringBench.createWithThreeAttributes
avgt 15 0.179 ± 0.001 us/op
AttributedStringBench.createWithThreeAttributes:gc.alloc.rate
avgt 15 4059.315 ± 32.313 MB/sec
AttributedStringBench.createWithThreeAttributes:gc.alloc.rate.norm
avgt 15 760.001 ± 0.001 B/op
AttributedStringBench.createWithThreeAttributes:gc.count
avgt 15 158.000 counts
AttributedStringBench.createWithThreeAttributes:gc.time
avgt 15 115.000 ms
AttributedStringBench.iterateMissingWithNoAttributes
avgt 15 0.064 ± 0.001 us/op
AttributedStringBench.iterateMissingWithNoAttributes:gc.alloc.rate
avgt 15 713.782 ± 13.008 MB/sec
AttributedStringBench.iterateMissingWithNoAttributes:gc.alloc.rate.norm
avgt 15 48.000 ± 0.001 B/op
AttributedStringBench.iterateMissingWithNoAttributes:gc.count
avgt 15 113.000 counts
AttributedStringBench.iterateMissingWithNoAttributes:gc.time
avgt 15 72.000 ms
AttributedStringBench.iterateMissingWithOneAttribute
avgt 15 0.309 ± 0.003 us/op
AttributedStringBench.iterateMissingWithOneAttribute:gc.alloc.rate
avgt 15 148.133 ± 1.549 MB/sec
AttributedStringBench.iterateMissingWithOneAttribute:gc.alloc.rate.norm
avgt 15 48.002 ± 0.001 B/op
AttributedStringBench.iterateMissingWithOneAttribute:gc.count
avgt 15 69.000 counts
AttributedStringBench.iterateMissingWithOneAttribute:gc.time
avgt 15 46.000 ms
AttributedStringBench.iterateMissingWithThreeAttributes
avgt 15 0.363 ± 0.002 us/op
AttributedStringBench.iterateMissingWithThreeAttributes:gc.alloc.rate
avgt 15 125.929 ± 0.678 MB/sec
AttributedStringBench.iterateMissingWithThreeAttributes:gc.alloc.rate.norm
avgt 15 48.003 ± 0.001 B/op
AttributedStringBench.iterateMissingWithThreeAttributes:gc.count
avgt 15 60.000 counts
AttributedStringBench.iterateMissingWithThreeAttributes:gc.time
avgt 15 46.000 ms
AttributedStringBench.iteratePresentWithOneAttribute
avgt 15 0.423 ± 0.002 us/op
AttributedStringBench.iteratePresentWithOneAttribute:gc.alloc.rate
avgt 15 108.245 ± 0.562 MB/sec
AttributedStringBench.iteratePresentWithOneAttribute:gc.alloc.rate.norm
avgt 15 48.003 ± 0.001 B/op
AttributedStringBench.iteratePresentWithOneAttribute:gc.count
avgt 15 51.000 counts
AttributedStringBench.iteratePresentWithOneAttribute:gc.time
avgt 15 40.000 ms
AttributedStringBench.iteratePresentWithThreeAttributes
avgt 15 0.452 ± 0.005 us/op
AttributedStringBench.iteratePresentWithThreeAttributes:gc.alloc.rate
avgt 15 101.156 ± 1.154 MB/sec
AttributedStringBench.iteratePresentWithThreeAttributes:gc.alloc.rate.norm
avgt 15 48.003 ± 0.001 B/op
AttributedStringBench.iteratePresentWithThreeAttributes:gc.count
avgt 15 48.000 counts
AttributedStringBench.iteratePresentWithThreeAttributes:gc.time
avgt 15 35.000 ms
After PR changes:
Benchmark
Mode Cnt Score Error Units
AttributedStringBench.createWithNoAttributes
avgt 15 0.003 ± 0.001 us/op
AttributedStringBench.createWithNoAttributes:gc.alloc.rate
avgt 15 10291.391 ± 309.539 MB/sec
AttributedStringBench.createWithNoAttributes:gc.alloc.rate.norm
avgt 15 32.000 ± 0.001 B/op
AttributedStringBench.createWithNoAttributes:gc.count
avgt 15 197.000 counts
AttributedStringBench.createWithNoAttributes:gc.time
avgt 15 186.000 ms
AttributedStringBench.createWithOneAttribute
avgt 15 0.033 ± 0.001 us/op
AttributedStringBench.createWithOneAttribute:gc.alloc.rate
avgt 15 8812.068 ± 269.579 MB/sec
AttributedStringBench.createWithOneAttribute:gc.alloc.rate.norm
avgt 15 304.000 ± 0.001 B/op
AttributedStringBench.createWithOneAttribute:gc.count
avgt 15 169.000 counts
AttributedStringBench.createWithOneAttribute:gc.time
avgt 15 169.000 ms
AttributedStringBench.createWithThreeAttributes
avgt 15 0.147 ± 0.002 us/op
AttributedStringBench.createWithThreeAttributes:gc.alloc.rate
avgt 15 4888.052 ± 66.603 MB/sec
AttributedStringBench.createWithThreeAttributes:gc.alloc.rate.norm
avgt 15 752.001 ± 0.001 B/op
AttributedStringBench.createWithThreeAttributes:gc.count
avgt 15 151.000 counts
AttributedStringBench.createWithThreeAttributes:gc.time
avgt 15 120.000 ms
AttributedStringBench.iterateMissingWithNoAttributes
avgt 15 0.063 ± 0.002 us/op
AttributedStringBench.iterateMissingWithNoAttributes:gc.alloc.rate
avgt 15 732.731 ± 27.288 MB/sec
AttributedStringBench.iterateMissingWithNoAttributes:gc.alloc.rate.norm
avgt 15 48.000 ± 0.001 B/op
AttributedStringBench.iterateMissingWithNoAttributes:gc.count
avgt 15 116.000 counts
AttributedStringBench.iterateMissingWithNoAttributes:gc.time
avgt 15 76.000 ms
AttributedStringBench.iterateMissingWithOneAttribute
avgt 15 0.215 ± 0.003 us/op
AttributedStringBench.iterateMissingWithOneAttribute:gc.alloc.rate
avgt 15 213.145 ± 2.898 MB/sec
AttributedStringBench.iterateMissingWithOneAttribute:gc.alloc.rate.norm
avgt 15 48.002 ± 0.001 B/op
AttributedStringBench.iterateMissingWithOneAttribute:gc.count
avgt 15 101.000 counts
AttributedStringBench.iterateMissingWithOneAttribute:gc.time
avgt 15 61.000 ms
AttributedStringBench.iterateMissingWithThreeAttributes
avgt 15 0.229 ± 0.002 us/op
AttributedStringBench.iterateMissingWithThreeAttributes:gc.alloc.rate
avgt 15 199.575 ± 2.030 MB/sec
AttributedStringBench.iterateMissingWithThreeAttributes:gc.alloc.rate.norm
avgt 15 48.002 ± 0.001 B/op
AttributedStringBench.iterateMissingWithThreeAttributes:gc.count
avgt 15 93.000 counts
AttributedStringBench.iterateMissingWithThreeAttributes:gc.time
avgt 15 59.000 ms
AttributedStringBench.iteratePresentWithOneAttribute
avgt 15 0.251 ± 0.010 us/op
AttributedStringBench.iteratePresentWithOneAttribute:gc.alloc.rate
avgt 15 182.271 ± 7.221 MB/sec
AttributedStringBench.iteratePresentWithOneAttribute:gc.alloc.rate.norm
avgt 15 48.002 ± 0.001 B/op
AttributedStringBench.iteratePresentWithOneAttribute:gc.count
avgt 15 85.000 counts
AttributedStringBench.iteratePresentWithOneAttribute:gc.time
avgt 15 56.000 ms
AttributedStringBench.iteratePresentWithThreeAttributes
avgt 15 0.252 ± 0.003 us/op
AttributedStringBench.iteratePresentWithThreeAttributes:gc.alloc.rate
avgt 15 181.707 ± 1.873 MB/sec
AttributedStringBench.iteratePresentWithThreeAttributes:gc.alloc.rate.norm
avgt 15 48.002 ± 0.001 B/op
AttributedStringBench.iteratePresentWithThreeAttributes:gc.count
avgt 15 85.000 counts
AttributedStringBench.iteratePresentWithThreeAttributes:gc.time
avgt 15 54.000 ms
> test/micro/org/openjdk/bench/java/text/AttributedStringBench.java line 90:
>
>> 88:
>> 89: @Benchmark
>> 90: public void iterationWithFourAttributes(Blackhole blackhole) {
>
> This might be better renamed, since `string3` is technically three attribute
> _keys_.
Thanks, test names have been updated per unique key count.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30402#discussion_r3220556185
PR Review Comment: https://git.openjdk.org/jdk/pull/30402#discussion_r3220555774
PR Review Comment: https://git.openjdk.org/jdk/pull/30402#discussion_r3220555099
PR Review Comment: https://git.openjdk.org/jdk/pull/30402#discussion_r3220552401
