On Wed, 6 May 2026 14:56:53 GMT, Naoto Sato <[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   
>> ...
>
> Thanks and apologies for missing this PR earlier. This looks like a solid 
> refactor to me. I did notice a few instances of “vectors” in the comments 
> that should probably be updated as well. I'll run our CI with these changes 
> to make sure everything is fine.

@naotoj Great, thanks for the review and the CI check!

I'll leave this PR open for another couple of days, in case any other reviewers 
want to have a look. If no other concerns are raised, I'll integrate at the end 
of the week.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/30402#issuecomment-4399013951

Reply via email to