On Tue, 20 Aug 2024 18:31:19 GMT, Chen Liang <li...@openjdk.org> wrote:
>> This PR reduces the amount of lambda forms (LFs) which are created when >> generating var handles for simple struct field accessors. This contributes >> to the startup regression seen in >> [JDK-8337505](https://bugs.openjdk.org/browse/JDK-8337505). >> >> There are essentially three sources of excessive var handle adaptation: >> >> 1. `LayoutPath::dereferenceHandle` has to do some very complex adaptation >> (including a permute) in order to inject alignment and size checks (against >> the enclosing layout) on the generated var handle. >> 2. Even in simple cases (e.g. when there's no dynamic coordinate), the >> offset of the accessed field is added to the var handle via an expensive >> collect adapter. >> 3. When we adapt a `long` var handle to work on `MemorySegment` using an >> `AddressLayout`, we make no distinction on whether the address layout has a >> target layout or not. In the latter case (common!) we can adapt more simply. >> >> The meat of this PR is to address (1) by changing the shape of the generated >> helpers in the `X-VarHandleSegmentView.java.template` class. That is, the >> method for doing a plain get will now have the following shape: >> >> >> T get(MemorySegment segment, MemoryLayout enclosing, long base, long offset) >> >> >> Where: >> * `segment` is the segment being accessed >> * `enclosing` is the enclosing layout (the root of the selected layout path) >> against which to check size and alignment >> * `base` is the public-facing offset passed by the user when calling `get` >> on the var handle >> * `offset` is the offset at which the selected layout element can be found >> from the root (this can be replaced with an expression that takes several >> dynamic indices and turn them into a single offset) >> >> With this organization, it is easy to see how, in order to create a memory >> access var handle for a struct field `S.f` we only need to: >> * inject the enclosing layout `S` into the var handle (into the `enclosing` >> coordinate) >> * inject the offset of `S.f` into the var handle (into the `offset` >> coordinate) >> >> This way, we get our plain old memory access var handle featuring only two >> coordinates: a segment and an offset. Note how, to get there, we only needed >> very simple adaptations (e.g. `MethodHandles::insertCoordinates`). >> >> #### Evaluation >> >> I did some tests using the benchmark in >> [JDK-8337505](https://bugs.openjdk.org/browse/JDK-8337505) to assess the >> impact of this change on startup. To evaluate startup, I ran the benchmark >> 50 times and then took some stats. Here's what the... > > src/java.base/share/classes/jdk/internal/foreign/Utils.java line 245: > >> 243: >> 244: @ForceInline >> 245: public static void checkEnclosingLayout(MemorySegment segment, long >> offset, MemoryLayout enclosing, boolean readOnly) { > > Can't the first argument be `AbstractMemorySegmentImpl`? The new call site > already has an `AbstractMemorySegmentImpl` and the private static method site > can do the cast instead. I suppose it could yes - any reason as to why moving the cast around is better? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20647#discussion_r1723984684
