perf: collapse importer exists+isDir into single isFile stat#872
Open
He-Pin wants to merge 1 commit into
Open
Conversation
## Motivation async-profiler on kube-prometheus shows file-I/O syscalls account for ~15% of CPU (top leaf frames: `__open` 294, `stat64` 210, `access` 167, `close` 102, `read` 92). The `SimpleImporter.resolve` chain does `os.exists(f) && !os.isDir(f)` per candidate — `os.exists` backs onto `Files.exists` (POSIX `access(F_OK)`) and `os.isDir` backs onto `Files.isDirectory` (POSIX `stat`). For an existent candidate that is two syscalls. The same check is repeated again in `readPath`, so the success path pays four syscalls just for the resolve+read gate before the real read happens. ## Modification Replace `os.exists(f) && !os.isDir(f)` with `os.isFile(_)` in both `SimpleImporter.resolve` and `readPath`. `os.isFile` delegates to `Files.isRegularFile`, which is one `stat` call and catches `IOException` internally — so it returns `false` for non-existent paths without throwing. Semantics are preserved for any path a Jsonnet import would care about (regular files, including those reached through symlinks). ## Result async-profiler confirms `access` leaf samples eliminated entirely (167 → 0). 3-run × 200-iter A/B on kube-prometheus (JVM build, fresh Interpreter per run, no asprof attached): | | baseline | after | delta | |--------|----------|--------|-------------------| | min | 54.5 ms | 52.6 ms| −1.9 ms (−3.5%) | | p50 | 58.1 ms | 55.8 ms| −2.3 ms (−4.0%) | | mean | 61.9 ms | 59.2 ms| −2.7 ms (−4.4%) | `stat64` count goes slightly up (210 → 305) because `Files.isRegularFile` uses `stat` instead of `access`, but the net syscall count drops ~72 per run. Scala.js importer is user-provided JS closures so it is unaffected; Scala Native picks up the same benefit since the file lives under `src-jvm-native`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
async-profiler on kube-prometheus shows file-I/O syscalls account for ~15% of CPU (top leaf frames:
__open294,stat64210,access167,close102,read92). TheSimpleImporter.resolvechain doesos.exists(f) && !os.isDir(f)per candidate —os.existsbacks ontoFiles.exists(POSIXaccess(F_OK)) andos.isDirbacks ontoFiles.isDirectory(POSIXstat). For an existent candidate that is two syscalls. The same check is repeated again inreadPath, so the success path pays four syscalls just for the resolve+read gate before the real read happens.Modification
Replace
os.exists(f) && !os.isDir(f)withos.isFile(_)in bothSimpleImporter.resolveandreadPath.os.isFiledelegates toFiles.isRegularFile, which is onestatcall and catchesIOExceptioninternally — so it returnsfalsefor non-existent paths without throwing. Semantics are preserved for any path a Jsonnet import would care about (regular files, including those reached through symlinks).Result
async-profiler confirms
accessleaf samples eliminated entirely (167 → 0). 3-run × 200-iter A/B on kube-prometheus (JVM build, freshInterpreterper run, no asprof attached):stat64count goes slightly up (210 → 305) becauseFiles.isRegularFileusesstatinstead ofaccess, but the net syscall count drops ~72 per run. Scala.js importer is user-provided JS closures so it is unaffected; Scala Native picks up the same benefit since the file lives undersrc-jvm-native.Test plan
./mill 'sjsonnet.jvm[3.3.7]'.compile./mill 'sjsonnet.native[3.3.7]'.compile./mill __.checkFormat./mill 'sjsonnet.jvm[3.3.7]'.test(full JVM suite passes)