RFR: 8321931: memory_swap_current_in_bytes reports 0 as "unlimited"

Thomas Stuefe stuefe at openjdk.org
Wed Jan 17 07:17:50 UTC 2024


On Tue, 16 Jan 2024 22:43:10 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> src/hotspot/os/linux/osContainer_linux.cpp line 142:
>> 
>>> 140: void OSContainer::print_container_helper(outputStream* st, jlong j, const char* metrics, boolean limit) {
>>> 141:   st->print("%s: ", metrics);
>>> 142:   if ((j > 0) || ((j == 0) && (limit == false))) {
>> 
>> Clearer would be a reversal like this:
>> 
>> if (j == 0 && limit) {
>>   "unlimited"
>> } else {
>>   print
>> }
>
> I think I decided to have it this way to draw the attention to the fact that the value `0` for a `non-limit` kind should be treated as numerical value and not a special case.
> 
> This provides, in my opinion, a self-documenting code without the need for an explicit comment. That was my rationale at least, but if that doesn't come across, then we can simplify and add an actual comment?

Up to you. Both are okay, this was just a small nit :)

>> src/hotspot/os/linux/osContainer_linux.hpp line 48:
>> 
>>> 46:   static void init();
>>> 47:   static void print_version_specific_info(outputStream* st);
>>> 48:   static void print_container_helper(outputStream* st, jlong j, const char* metrics, boolean limit = false);
>> 
>> Can we name this parameter better, or add a comment at least?
>
> Would `isLimit` be any better?

Yes

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17314#discussion_r1454780928
PR Review Comment: https://git.openjdk.org/jdk/pull/17314#discussion_r1454781179


More information about the hotspot-runtime-dev mailing list