[lworld] RFR: 8251046: [lworld] [lw3] C1 should avoid heap allocations in withfield when possible
Tobias Hartmann
thartmann at openjdk.java.net
Thu Aug 6 14:11:43 UTC 2020
On Wed, 5 Aug 2020 13:14:12 GMT, Frederic Parain <fparain at openjdk.org> wrote:
> Please review this patch which reduces the number of heap allocations when executing the withfield bytecode with C1.
> The CR linked below contains a more detailed description of the optimization:
>
> CR: https://bugs.openjdk.java.net/browse/JDK-8251046
>
> A rudimentary benchmark shows encouraging numbers:
> Measuring inline type creation time with -XX:TieredStopAtLevel=1
> Inline class Point has two int fields.
> Inline class Rectangle has four int fields.
>
> Without the optimization:
> Benchmark Mode Samples Score Score error Units
> o.s.MyBenchmark.testPointCreation avgt 200 7.573 0.027 ns/op
> o.s.MyBenchmark.testRectangleCreation avgt 200 21.387 0.067 ns/op
>
> With the optimization:
> Benchmark Mode Samples Score Score error Units
> o.s.MyBenchmark.testPointCreation avgt 200 5.284 0.014 ns/op
> o.s.MyBenchmark.testRectangleCreation avgt 200 7.750 0.082 ns/op
>
> Tested manually and with Mach5, tiers 1 to 3 (with Tobias' changes enabling more C1 testing).
>
> Thank you,
>
> Fred
Overall looks good to me, nice improvement! I've added some comments to the code.
src/hotspot/share/c1/c1_Instruction.cpp line 422:
> 421:
> 422: StoreField::StoreField(Value obj, int offset, ciField* field, Value value, bool is_static,
> 423: ValueStack* state_before, bool needs_patching)
Indentation should be fixed.
src/hotspot/share/c1/c1_Instruction.cpp line 438:
> 437:
> 438: StoreIndexed::StoreIndexed(Value array, Value index, Value length, BasicType elt_type, Value value, ValueStack*
> state_before, 439: bool check_boolean, bool mismatched)
Indentation should be fixed.
src/hotspot/share/c1/c1_GraphBuilder.cpp line 1107:
> 1106: { Value w = state()->raw_pop();
> 1107: NewInlineTypeInstance::update_stack_count(w);
> 1108: }
Wouldn't it be cleaner to use virtual calls on the Value class here?
src/hotspot/share/c1/c1_Instruction.hpp line 1419:
> 1418: virtual void set_not_larva_anymore() {
> 1419: _in_larval_state = false; }
> 1420:
No newline needed after "{" or add newline before "}" as well.
-------------
Changes requested by thartmann (Committer).
PR: https://git.openjdk.java.net/valhalla/pull/137
More information about the valhalla-dev
mailing list