On Fri, 28 Feb 2025 15:57:18 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 28 Feb 2025 15:57:18 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 28 Feb 2025 15:57:18 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by 47%:
>
> long[] arr = new long[8];
> var ms = Memory
On Fri, 28 Feb 2025 10:37:17 GMT, Maurizio Cimadamore
wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> noStride -> constantOffset, optimize VH classes to have only 2 instead of
>> 3 classes for each type
>
> src/jav
On Fri, 28 Feb 2025 10:21:27 GMT, Maurizio Cimadamore
wrote:
>> Maybe something like `isOffsetFixed` or `hasFixedOffset` would be better?
>
> (I suggested renaming to something that didn't contain the term "offset" --
> it's a subjective thing and if I'm in the minority that's ok. Maybe
> `noI
On Fri, 28 Feb 2025 10:38:28 GMT, Maurizio Cimadamore
wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> noStride -> constantOffset, optimize VH classes to have only 2 instead of
>> 3 classes for each type
>
> src/jav
On Thu, 27 Feb 2025 14:17:27 GMT, Jorn Vernee wrote:
>> Well, the reason I removed the eager init is that their creation in clinit
>> is super costly. Also I think one pair of getter + creator is better than 3
>> pairs.
>
>> ... their creation in clinit is super costly
>
> You mean because th
On Fri, 28 Feb 2025 10:29:46 GMT, Maurizio Cimadamore
wrote:
> I have observed the same thing -- these helper method handles lead to some
> bytecode spinning
Although - that was before; in the new code these helpers don't do
adaptation... so it should be ok. (I also see the code has been upda
On Thu, 27 Feb 2025 16:00:47 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Tue, 25 Feb 2025 14:39:23 GMT, Magnus Ihse Bursie wrote:
>> Left a space and an extra note to make the comment hash more obvious.
>
> Unless you plan to shortly push a new PR where you either enable this
> functionality, or remove the commented-out lines, I strongly prefer *not* to
> have co
On Tue, 25 Feb 2025 09:51:08 GMT, Maurizio Cimadamore
wrote:
>> Benchmark results for the latest revision appears performance neutral.
>> bytestacks same as last revision. 11 jobs left in tier 1-3, no failure so
>> far. Also created CSR for this minor behavioral change.
>>
>> Benchmark
On Thu, 27 Feb 2025 14:21:17 GMT, Jorn Vernee wrote:
>> In last revision I called it `fixedOffset`, but it becomes confusing with
>> the actual fixed value of the offset.
>
> Maybe something like `isOffsetFixed` or `hasFixedOffset` would be better?
(I suggested renaming to something that didn't
On Thu, 27 Feb 2025 16:00:47 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by 47%:
>
> long[] arr = new long[8];
> var ms = Memory
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by 47%:
>
> long[] arr = new long[8];
> var ms = Memory
On Wed, 26 Feb 2025 19:53:45 GMT, Chen Liang wrote:
>> src/java.base/share/classes/java/lang/invoke/X-VarHandleSegmentView.java.template
>> line 83:
>>
>>> 81: bb.unsafeGetBase(),
>>> 82: offset(bb, base, offset),
>>> 83: handle.be);
>
On Wed, 26 Feb 2025 19:54:59 GMT, Chen Liang wrote:
>> I suggest maybe renaming `noStride` to something like
>> `fixedOffsetInEnclosing`
>
> In last revision I called it `fixedOffset`, but it becomes confusing with the
> actual fixed value of the offset.
Maybe something like `isOffsetFixed` or
On Thu, 27 Feb 2025 13:24:16 GMT, Chen Liang wrote:
> ... creation in clinit is super costly
You mean because threads can not race to initialize? I'd think the extra walks
to create 3 lookups might offset that though...
-
PR Review Comment: https://git.openjdk.org/jdk/pull/23720#
On Thu, 27 Feb 2025 10:26:29 GMT, Jorn Vernee wrote:
>> This emulates how MethodHandleImpl does the cache.
>
> Ok. I think the benefit is that each element is individually lazy
> initialized. I'm not sure about the setup with the array though. It seems
> like having 3 stable fields would be sim
On Wed, 26 Feb 2025 19:56:04 GMT, Chen Liang wrote:
>> src/java.base/share/classes/jdk/internal/foreign/Utils.java line 74:
>>
>>> 72: return ret;
>>> 73: return computeFilterHandle(index);
>>> 74: }
>>
>> Why is this using an array, instead of just having 3 fields?
>
>
On Wed, 26 Feb 2025 20:13:42 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Wed, 26 Feb 2025 20:13:42 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Wed, 26 Feb 2025 17:26:41 GMT, Jorn Vernee wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by 47%:
>
> long[] arr = new long[8];
> var ms = Memory
On Wed, 26 Feb 2025 17:23:02 GMT, Jorn Vernee wrote:
>> src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java line 216:
>>
>>> 214: } else {
>>> 215: // simpler adaptation
>>> 216: handle = MethodHandles.insertCoordinates(handle, 2,
>>> offset);
On Wed, 26 Feb 2025 17:18:16 GMT, Jorn Vernee wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> src/java.base/share/classes/java/lang/invoke/X-VarHandleSegmentView.j
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Wed, 26 Feb 2025 17:22:13 GMT, Jorn Vernee wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java line
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Wed, 26 Feb 2025 16:59:39 GMT, Jorn Vernee wrote:
>> src/java.base/share/classes/java/lang/invoke/VarForm.java line 150:
>>
>>> 148: String methodName = value.methodName();
>>> 149: MethodType type =
>>> methodType_table[value.at.ordinal()].insertParameterTypes(0,
>>> VarHan
On Fri, 21 Feb 2025 10:05:36 GMT, Maurizio Cimadamore
wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> src/java.base/share/classes/java/lang/invoke/VarForm.java li
On Tue, 25 Feb 2025 09:51:08 GMT, Maurizio Cimadamore
wrote:
>> Benchmark results for the latest revision appears performance neutral.
>> bytestacks same as last revision. 11 jobs left in tier 1-3, no failure so
>> far. Also created CSR for this minor behavioral change.
>>
>> Benchmark
On Tue, 25 Feb 2025 23:11:30 GMT, Chen Liang wrote:
> Indeed, the byte access currently only supports read/write (automatically
> aligned). No CAS or bitwise for now in FFM.
Do you know why that is so?
-
PR Comment: https://git.openjdk.org/jdk/pull/23720#issuecomment-2683738533
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 21 Feb 2025 20:17:09 GMT, Chen Liang wrote:
>>> I have disabled them with #, and the status is confirmed by test for access
>>> modes. I kept the infra to make future reenabling easy.
>>
>> Doh - I missed the `#` -- maybe add few more to make that more explicit? (I
>> agree with the ap
On Tue, 25 Feb 2025 10:02:51 GMT, Per Minborg wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> src/java.base/share/classes/java/lang/invoke/SegmentVarHandle.java li
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Mon, 24 Feb 2025 17:53:36 GMT, Chen Liang wrote:
>> Maybe let's leave it, at least in this round
>
> Done in latest update.
Thanks! I think it looks very good!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/23720#discussion_r1969396191
On Mon, 24 Feb 2025 19:11:23 GMT, Chen Liang wrote:
> I think direct var handles are quite cheap already and don't really need
> caching. What really worth caching should be the combinators and the VH
> derived from those combinators.
Right -- and one big advantage of this PR, if I'm correct,
On Fri, 21 Feb 2025 22:19:08 GMT, Chen Liang wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> Benchmark results for the latest revision appears performance neutral.
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 21 Feb 2025 10:15:12 GMT, Maurizio Cimadamore
wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> src/java.base/share/classes/java/lang/invoke/X-VarHandleSegm
On Mon, 24 Feb 2025 11:34:15 GMT, Maurizio Cimadamore
wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> src/java.base/share/classes/jdk/internal/foreign/Utils.java
On Fri, 21 Feb 2025 18:37:47 GMT, Maurizio Cimadamore
wrote:
>> Now this will require code update to static import `offset` from
>> `VarHandleSegmentViewBase`. Is that acceptable?
>
> Maybe let's leave it, at least in this round
Done in latest update.
>> This is a one-time cold path initializ
On Fri, 21 Feb 2025 18:54:24 GMT, Chen Liang wrote:
>> I'm not convinced about the one in ValueLayouts *e.g. where this comment was
>> added) -- if you agree that the other cache should already handle this --
>> then maybe we can only keep one
>
> Indeed, the other cache already handles this.
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 21 Feb 2025 22:12:43 GMT, Chen Liang wrote:
>> src/java.base/share/classes/jdk/internal/foreign/Utils.java line 120:
>>
>>> 118: * with a {@link ValueLayout#varHandle()} call is cached inside a
>>> stable field of the value layout implementation.
>>> 119: * This optimizes comm
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 21 Feb 2025 22:11:01 GMT, Maurizio Cimadamore
wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review remarks, dates, some more simplifications
>
> src/java.base/share/classes/jdk/internal/foreign/Utils.java
On Fri, 21 Feb 2025 20:14:19 GMT, Chen Liang wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a simple ma
On Fri, 21 Feb 2025 14:43:31 GMT, Maurizio Cimadamore
wrote:
>> I have disabled them with #, and the status is confirmed by test for access
>> modes. I kept the infra to make future reenabling easy.
>
>> I have disabled them with #, and the status is confirmed by test for access
>> modes. I ke
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by 47%:
>
> long[] arr = new long[8];
> var ms = Memory
On Fri, 21 Feb 2025 18:34:48 GMT, Maurizio Cimadamore
wrote:
>> We have two caches, one in `ValueLayouts` and another in `Utils`. Should we
>> remove both?
>
> I'm not convinced about the one in ValueLayouts *e.g. where this comment was
> added) -- if you agree that the other cache should alre
On Fri, 21 Feb 2025 16:04:57 GMT, Chen Liang wrote:
>> src/java.base/share/classes/java/lang/invoke/X-VarHandleSegmentView.java.template
>> line 77:
>>
>>> 75: }
>>> 76:
>>> 77: @ForceInline
>>
>> Question: can this and the `offset` method go in the
>> `VarHandleSegmentViewBa
On Fri, 21 Feb 2025 10:12:45 GMT, Maurizio Cimadamore
wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a
On Fri, 21 Feb 2025 13:53:47 GMT, Chen Liang wrote:
>> src/java.base/share/classes/jdk/internal/foreign/Utils.java line 77:
>>
>>> 75:
>>> 76: private static MethodHandle computeFilterHandle(int index) {
>>> 77: MethodHandles.Lookup lookup = MethodHandles.lookup();
>>
>> please bre
On Fri, 21 Feb 2025 13:49:51 GMT, Chen Liang wrote:
> I have disabled them with #, and the status is confirmed by test for access
> modes. I kept the infra to make future reenabling easy.
Doh - I missed the `#` -- maybe add few more to make that more explicit? (I
agree with the approach)
>> s
On Fri, 21 Feb 2025 09:59:27 GMT, Maurizio Cimadamore
wrote:
>> Simplify the layout access var handles to be direct in some common cases.
>> Also made `VarHandle::isAccessModeSupported` report if an access mode is
>> supported for a VH.
>>
>> Reduces the instructions to execute this code in a
On Fri, 21 Feb 2025 00:01:36 GMT, Chen Liang wrote:
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by
On Fri, 21 Feb 2025 11:05:30 GMT, Maurizio Cimadamore
wrote:
> Overall, this is a great cleanup and consolidation of the memory segment VH
> code. I left various comments -- but this is already in a great shape. Thanks!
It would be useful to run some micro benchmarks before/after -- especially
On Fri, 21 Feb 2025 00:01:36 GMT, Chen Liang wrote:
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by
On Fri, 21 Feb 2025 00:01:36 GMT, Chen Liang wrote:
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by
On Fri, 21 Feb 2025 00:01:36 GMT, Chen Liang wrote:
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by
On Fri, 21 Feb 2025 00:01:36 GMT, Chen Liang wrote:
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by
On Fri, 21 Feb 2025 00:01:36 GMT, Chen Liang wrote:
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by
On Fri, 21 Feb 2025 00:01:36 GMT, Chen Liang wrote:
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by
On Fri, 21 Feb 2025 00:01:36 GMT, Chen Liang wrote:
> Simplify the layout access var handles to be direct in some common cases.
> Also made `VarHandle::isAccessModeSupported` report if an access mode is
> supported for a VH.
>
> Reduces the instructions to execute this code in a simple main by
Simplify the layout access var handles to be direct in some common cases. Also
made `VarHandle::isAccessModeSupported` report if an access mode is supported
for a VH.
Reduces the instructions to execute this code in a simple main by 47%:
long[] arr = new long[8];
var ms = MemorySegment.ofArray(
73 matches
Mail list logo
