JDK-8268622 - Performance issues in javac `Name` class

[Jonathan Gibbons]

On 3/3/23 6:23 AM, Archie Cobbs wrote:
> On Thu, Mar 2, 2023 at 5:32 PM Archie Cobbs <archie.cobbs at gmail.com> 
> wrote:
>
>     On Tue, Feb 28, 2023 at 9:30 AM Archie Cobbs
>     <archie.cobbs at gmail.com> wrote:
>
>         Jon and I have been discussing JDK-8268622
>         <https://bugs.openjdk.org/browse/JDK-8268622> via JBS
>         comments, but it's better off being discussed on compiler-dev,
>         so we're moving the discussion here...
>
>
>     Jon (and anyone else interested),
>
>     Please take a look at https://github.com/openjdk/jdk/pull/12843
>     and let me know what you think.
>
>
> Also here's a "sketch" of a refactoring along the lines of JDK-8269957 
> <https://bugs.openjdk.org/browse/JDK-8269957> "facilitate alternate 
> impls of NameTable and Name".
>
> https://github.com/openjdk/jdk/compare/master...archiecobbs:jdk:JDK-8269957
>
> Let me know if this is the kind of thing you had in mind. Note, this 
> includes the above patch as well.
>
> Overview of changes...
>
>   * Remove any byte-oriented methods from Name and NameTable API's,
>     except for those that "import" or "export" as UTF-8.
>   * Refactor SharedNameTable & UnsharedNameTable to have a new common
>     superclass Utf8NameTable
>   * Fix bug in PoolReader, in that it was not doing any validation
>     when decoding "Modified UTF-8". For example, this bug could result
>     in duplicate Names if a classfile contained the same name encoded
>     two different ways.
>   * Eliminated an unnecessary extra array allocation in
>     Convert.utf2chars()
>   * Rename Name.lastIndexOf() -> Name.lastIndexOfAscii() for safety
>
> -Archie
>
> -- 
> Archie L. Cobbs


Archie,

This is certainly along the lines I imagined, especially your first 
bullet, albeit much more thorough and in depth than I had previously 
considered.

I would give you inline code comments, except that it's not a PR yet.  I 
note that I generally distrust the `getMessage` for any exception for 
which the message is not formally specified in some way ... in other 
words, don't assume that `e.getMessage()` by itself is interesting.

Is it possible to write a test for the bug fix in PoolReader? What is an 
example of a name encoded in two different ways?

Although conceptually simple, this is a significant change for a very 
low level data type. It would be worth doing more testing than just the 
usual langtools tests.  For example, if you build JDK before and after 
this change, are the generated class files the same?

-- Jon




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/compiler-dev/attachments/20230303/7e40662d/attachment-0001.htm>