On Wed, 6 May 2026 15:56:21 GMT, Daniel Gredler <[email protected]> wrote:
>> While profiling some code which uses `LineBreakMeasurer`, I noticed that a
>> non-trivial amount of time was being spent in `AttributedString`, because
>> iterating through an `AttributedString` and getting a specific attribute for
>> each character ends up calling `Vector.indexOf` inside of
>> `AttributedString.getAttribute`. This boils down to the data structure
>> choice of storing run attribute data in two arrays of `Vector`s, rather than
>> in a single array of `Map`s. Based on testing, it looks like a single array
>> of `Map`s would be simpler, faster, and require slightly less memory in some
>> cases.
>>
>> A JMH benchmark is included in this PR. With the updated code I'm seeing a
>> 35% to 40% reduction in runtime for the iteration benchmark tests with one
>> or more attributes. I checked memory usage by enabling GC profiling and
>> looking at the three creation benchmark tests; it seems like there is no
>> difference when the string has no attributes, a 20% reduction in memory use
>> when the string has a single attribute, and a very small 1% reduction in
>> memory use when we set four attributes. Full JMH results below (I focused on
>> the `us/op` and `B/op` results specifically).
>>
>> Tests run:
>> - make test TEST="jtreg:test/jdk/java/text"
>> - make test TEST="jtreg:test/jdk/java/awt/font"
>> - make test TEST="jtreg:test/jdk/java/awt/Graphics2D/DrawString"
>> - make test TEST="micro:java.text.AttributedStringBench"
>> MICRO="OPTIONS=-prof gc"
>>
>> I've assumed that the existing tests provide adequate regression test
>> coverage, but there may be other Oracle-internal tests that should also be
>> run to verify correctness.
>>
>> JMH results **before** PR:
>>
>>
>> Benchmark Mode
>> Cnt Score Error Units
>> AttributedStringBench.creationWithFourAttributes avgt
>> 15 0.182 ± 0.004 us/op
>> AttributedStringBench.creationWithFourAttributes:gc.alloc.rate avgt
>> 15 3989.259 ± 85.345 MB/sec
>> AttributedStringBench.creationWithFourAttributes:gc.alloc.rate.norm avgt
>> 15 760.001 ± 0.001 B/op
>> AttributedStringBench.creationWithFourAttributes:gc.count avgt
>> 15 151.000 counts
>> AttributedStringBench.creationWithFourAttributes:gc.time avgt
>> 15 116.000 ms
>> AttributedStringBench.creationWithNoAttributes avgt
>> 15 0.003 ± 0.001 us/op
>> AttributedStringBench.creationWithNoAttributes:gc.alloc.rate avgt
>> ...
>
> 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.
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.
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.
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_.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30402#discussion_r3210832561
PR Review Comment: https://git.openjdk.org/jdk/pull/30402#discussion_r3210716797
PR Review Comment: https://git.openjdk.org/jdk/pull/30402#discussion_r3210712996
PR Review Comment: https://git.openjdk.org/jdk/pull/30402#discussion_r3210645781
