On Sun, 29 Sep 2024 13:35:14 GMT, Shaojin Wen <s...@openjdk.org> wrote:
>> Some DirectCodeBuilder related optimizations to improve startup and running
>> performance:
>> 1. Merge calls, merge writeU1 and writeU2 into writeU3
>> 2. Merge calls, merge writeU1 and writeIndex operations
>> 3. Directly use writeU1 instead of writeBytecode
>> 4. Rewrite the implementation of load and store
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> use array instead of ArrayList
src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java line
124:
> 122: }
> 123:
> 124: public void writeU3(int u1, int u2) {
This one should be `writeU1U2` or `write12`. If people use this as "write u2
and write u1" the higher byte of 1st arg will be truncated incorrectly.
The other `writeU2` and `writeU3` already have 3 args so it's easy to assume
that they drop the more significant bytes and only use the least significant
byte of each argument.
src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java line
144:
> 142: }
> 143:
> 144: public void writeU4(int x1, int x2) {
Since we are writing 2 u2 instead of 1 u4, we should call this `writeU2U2` or
just `write22` (unsigned or signed are the same). Same concern for byte
truncation.
src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java
line 605:
> 603: int count) {
> 604: bytecodesBufWriter.writeIndex(opcode.bytecode(), ref);
> 605: bytecodesBufWriter.writeU2(count << 8);
Suggestion:
bytecodesBufWriter.writeU2(count, 0);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21243#discussion_r1779821432
PR Review Comment: https://git.openjdk.org/jdk/pull/21243#discussion_r1779821071
PR Review Comment: https://git.openjdk.org/jdk/pull/21243#discussion_r1779821719
