On Mon, 12 Jun 2023 09:20:09 GMT, Doug Simon <dnsi...@openjdk.org> wrote:
>> src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/services/Services.java line
>> 314:
>>
>>> 312: String key = toJavaString(unsafe, unsafe.getLong(prop +
>>> keyOffset));
>>> 313: long valueAddress = unsafe.getLong(prop + valueOffset);
>>> 314: if (valueAddress != 0) {
>>
>> Is the lazy value construction really necessary? It seems like it requires
>> quite a bit of extra machinery.
>
> In big apps, I image there can be quite some large system property values
> (e.g. `java.class.path`) so we don't want to pay memory overhead per libgraal
> isolate for such properties as they're ignored by libgraal.
>
> I realized I can reduce the extra machinery based on the fact that
> `SystemProperties` is unmodifiable:
> https://github.com/openjdk/jdk/pull/14291/commits/6c346d35923402fb5d6ae0ef83214f8f0ca07b69
> The only method that still eager converts the string is
> `SystemProperties.values()` but that's fine as it's not used by JVMCI or
> Graal.
> I think the trade off is now acceptable?
Yes I like this trade off a bit better.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14291#discussion_r1226857200
