On Mon, 5 Jun 2023 09:02:06 GMT, Aleksandar Pejovic <apejo...@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/yr5jvhhsucrbt...@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