RFR: 8300645: Revert julong changes from 8194232 after 8292083

[Ioi Lam]

On Tue, 14 Feb 2023 18:06:38 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>>> I argue we don't care. max_jlong is 9,223,372,036,854,775,807
>> 
>> We do care if such a value can possibly be encountered. We don't want the VM to behave incorrectly. With the current implementation, it's hard to understand what happens when such a large value is encountered.
>> 
>> (In fact, the new behavior is wrong - the function will return -2 without considering `hier_memlimit`).
>> 
>> The problem is there's too much mental exercise when reading the code. There's casting, overflow/underflow, handling of special values of -1 and -2, and considerations for logging. Also, the same logic is duplicated in several places, so they can easily get out of sync in the future.
>> 
>> Is it possible to localize the logic to a utility function? 
>> 
>> Maybe something like this? All the casting and logging, etc, will be handled inside, in a single place.
>> 
>> 
>> // Returns -1 if the limit is not specified, or there was a failure when reading the limit, or if the specified
>> // limit is the same or larger than MIN2(physical_max, jlong_max).
>> // Otherwise returns a non-negative value that's smaller than MIN2(physical_max, jlong_max).
>> jlong limit = read_memory_limit("/memory.memsw.limit_in_bytes", physical_max);
>> 
>> 
>> `physical_max` will be a `julong`:
>> 
>> 
>> julong physical_max = os::Linux::physical_memory(); 
>> - or -
>> julong physical_max = os::Linux::host_swap() + os::Linux::physical_memory();
>
> @iklam After some thought, moving to a utility would need to move it into a utility preprocessor `macro` since it uses `GET_CONTAINER_INFO` macros. Would you prefer that over the current version? Or is that not the way to go? Just checking..

My suggestion is to first fix the `GET_CONTAINER_INFO` macros so that the logging works for julong types. That would address the main concern of this PR. How about this?

https://github.com/openjdk/jdk/compare/master...iklam:jdk:temp-improve-get-container-info-macros?expand=1

(the above changeset also simplifies `GET_CONTAINER_INFO_CPTR` so you don't need to specify the string length twice).

After that, if you want to consolidate the current logic to an utility function in a separate PR, that's fine too, but I think the current logic is correct (although a bit verbose).

-------------

PR: https://git.openjdk.org/jdk/pull/12166