RFR: 8309191: Reduce JDK dependencies of cgroup support
Severin Gehwolf
sgehwolf at openjdk.org
Mon Jun 5 09:47:06 UTC 2023
On Mon, 5 Jun 2023 09:02:06 GMT, Aleksandar Pejovic <apejovic at openjdk.org> wrote:
>> src/java.base/linux/classes/jdk/internal/platform/CgroupInfo.java line 110:
>>
>>> 108: */
>>> 109: static CgroupInfo fromCgroupsLine(String line) {
>>> 110: String[] tokens = line.split("\t");
>>
>> With this change, we now hard-code the expected delimiter and, thus, depend on what the kernel does. Do we have sufficient evidence this hasn't changed/won't change in the future?
>
> As far as I can tell, the delimiter hasn't changed since the file was introduced, and judging by the kernel mailing list (e.g., see [the following](https://lore.kernel.org/all/Yr5JVHhSUCrbT8OH@mtj.duckdns.org/)), I don't think it will change any time soon.
I'm not convinced this is a good change. Going by that mailing list thread, it suggests that people considered changing it. If one of those attempts were successful, it would have broken this code. It makes it rather fragile. The issue, with container detection code going wrong is that you most likely never notice. Translating this to GraalVM means that the native image, would a) detect the wrong version in use or b) fail detection and use host values. In both cases the application will likely misbehave in a container setup with resource limits applied and you won't (easily) know why. So even though it's unlikely to be a problem, there is a chance it could be and it's asking for trouble for no good reason.
Therefore, being conservative about delimiters makes sense here. My choice in this case would be more robust code over relying on external factors. YMMV.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14216#discussion_r1217815148
More information about the core-libs-dev
mailing list