[PING?] RFR(s): 8247863: Unreachable code in OperatingSystemImpl.getTotalSwapSpaceSize()

[Baesken, Matthias]

Hello Severin ,  

> the new cgroups implementation
> supporting v1 and v2 Metrics.getMemoryAndSwapLimit() will never return   0

Wouldn’t it be possible that the  coding  of  getMemoryAndSwapLimit   returns a negative value  (might not happen on a "healthy" system but you never know) :

jdk/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1Subsystem.java

444    public long getMemoryAndSwapLimit() {
445        long retval = getLongValue(memory, "memory.memsw.limit_in_bytes");
446        if (retval > CgroupV1SubsystemController.UNLIMITED_MIN) {
447            if (memory.isHierarchical()) {
448                // memory.memsw.limit_in_bytes returned unlimited, attempt
449                // hierarchical memory limit
450                String match = "hierarchical_memsw_limit";
451                retval = CgroupV1SubsystemController.getLongValueMatchingLine(memory,
452                                                            "memory.stat",
453                                                            match);
454            }
455        }
456        return CgroupV1SubsystemController.longValOrUnlimited(retval);
457    }


jdk/src/java.base/linux/classes/jdk/internal/platform/cgroupv2/CgroupV2Subsystem.java

278    public long getMemoryAndSwapLimit() {
279        String strVal = CgroupSubsystemController.getStringValue(unified, "memory.swap.max");
280        return limitFromString(strVal);
281    }

So  the check you want to clean up does no harm, and might handle "strange" cases - why not keep it?



Best regards, Matthias


-----Original Message-----
From: Severin Gehwolf <sgehwolf at redhat.com> 
Sent: Mittwoch, 15. Juli 2020 11:47
To: core-libs-dev <core-libs-dev at openjdk.java.net>; serviceability-dev <serviceability-dev at openjdk.java.net>
Cc: Baesken, Matthias <matthias.baesken at sap.com>; Bob Vandette <bob.vandette at oracle.com>
Subject: Re: [PING?] RFR(s): 8247863: Unreachable code in OperatingSystemImpl.getTotalSwapSpaceSize()

Anyone?

On Mon, 2020-06-29 at 17:53 +0200, Severin Gehwolf wrote:
> Hi,
> 
> Could I please get a review of this dead-code removal? During review of
> JDK-8244500 it was discovered that with the new cgroups implementation
> supporting v1 and v2 Metrics.getMemoryAndSwapLimit() will never return
> 0 when relevant cgroup files are missing. E.g. on a system where the
> kernel doesn't support swap limit capabilities. Therefore this code
> introduced with JDK-8236617 can no longer be reached and should get
> removed.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8247863
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8247863/01/webrev/
> 
> Testing: Matthias tested this on the affected system and it did pass
> for him. Docker tests on cgroup v1 and cgroup v2.
> 
> Thanks,
> Severin