Skip to content

List pack index directories from a map built once, which makes javac's first compile about a quarter faster - #34

Merged
Project516 merged 5 commits into
masterfrom
p516/index-children
Nov 4, 2026
Merged

Project516 merged 5 commits into
masterfrom
p516/index-children

Conversation

@Project516

Copy link
Copy Markdown
Owner

javac's first compile spent a fifth of its time listing directories. Its file manager opens the platform classes through the jrt: file system. Over the exploded image, jdk.internal.jrtfs.ExplodedImage walks every module directory: about 900 directories and 21,000 files. Each listing went through listIndex, which scanned every path in the pack index to find one directory's children.

  • PackIndex.children() builds a directory-to-names map in one pass and reuses it until the index grows. Entries are only ever added, when a module's lines are parsed, so a change in size is the signal to rebuild. Names keep index order, as before.
  • JVM.slotOf, which natives use to read and write fields by name, looked up a key built as owner + "." + name on every call. It now looks up the owner, then the name, in nested maps. Both are literal strings with cached hashes, so no key string is built. fillStat names its time fields as literals for the same reason.
  • The pack reader shares one TextDecoder (from the review of Parse the boot index of each module the first time a path in it is read #31).

Measured with com.sun.tools.javac.Main -d out Hi.java on dist/node.js, starting from a boot snapshot, on a small ARM board:

Case Before After
javac, one small file, Node 10.3 s 7.9 s
listIndex self time in that run 2.08 s none
Browser self-test, first compile step 8.2 to 9.2 s 6.3 to 6.8 s

The field lookup change alone is about 1% of the javac run (7.94 s to 7.87 s, median of 4); the rest is the directory map. The "before" column was measured before #31 too, so it includes that change's smaller gain.

Checks:

Noticed, not fixed:

  • The image walk itself still costs about 1.2 s of the javac run, mostly lstat through the NIO natives. It is the JDK's own ExplodedImage code, which HotSpot runs the same way on an exploded image.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Performance

    • Directory listings now load more efficiently, especially when browsing large indexes repeatedly.
    • Pack data is processed with less repeated setup.
  • Bug Fixes

    • File metadata now reports access, modification, and change times in the expected fields.
    • Missing Java fields now produce clearer error messages identifying the class and field.

The changes update browser index decoding and directory listing, JVM field-slot caching, and NIO timestamp assignment.

Changes: Browser index updates

Layer / File(s) Summary
Index decoding and directory listing
src/browser/http.ts
Pack and module index decoding reuse a module-level UTF-8 decoder. PackIndex caches directory child names, and listIndex uses the cached names.

JVM field-slot cache

Layer / File(s) Summary
Field-slot lookup and caching
src/jvm.ts
JVM.slots and slotOf use nested maps keyed by owner class and field name. Missing-field errors identify the owner and field separately.

NIO timestamps

Layer / File(s) Summary
Stat timestamp assignment
src/natives/nio.ts
fillStat assigns seconds and nanoseconds to the st_* timestamp fields using separate field-name lists.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related issues

Possibly related PRs

  • Project516/java-web#31 — Introduced PackIndex and listIndex; this PR adds cached directory children to that path.
  • Project516/java-web#4 — Introduced listIndex directory listing; this PR changes that path to use cached children.

Suggested reviewers: project516-review-bot

Merge Risk: Minimal · up to 5f9bb

The changes preserve the existing behavior of the reviewed paths.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly describes the main optimization: listing pack index directories from a reusable map to improve javac compile time.
Description check Passed The description is directly related to the changeset. It explains the directory map, nested field lookup, shared TextDecoder, performance results, and test results.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

A rabbit taps the index tree,
One decoder hums with quiet glee.
Field slots nest in maps so neat,
Stat times land where fields should meet.
The rabbit hops and checks the flow,
Then nibbles carrots, pleased to go.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: fed8f510-4d18-40fe-8564-0c07a86bc5a1
Commits

Reviewing files that changed from the base of the PR and between 37f9f79 and 5f9bb23.

Files selected for processing: 3
  • src/browser/http.ts
  • src/jvm.ts
  • src/natives/nio.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Recent review details
Context from checks skipped due to timeout: 6
  • GitHub Check: CodeRabbit
  • GitHub Check: jtreg / run (2)
  • GitHub Check: jtreg / run (1)
  • GitHub Check: jtreg / run (4)
  • GitHub Check: jtreg / run (3)
  • GitHub Check: test
Additional context used
Code guidelines: 1
AGENTS.md — auto-discovered
Path-based instructions: 1
Source excerpt: Compatibility first.

CodeRabbit inference engine (AGENTS.md)

Files:

  • src/natives/nio.ts
  • src/jvm.ts
  • src/browser/http.ts
Additional comments: 4
src/jvm.ts: 1

120-121: LGTM!

Also applies to: 720-721, 724-725, 727-727

src/natives/nio.ts: 1

91-92: LGTM!

src/browser/http.ts: 2

17-18: LGTM!

Also applies to: 32-32, 42-42, 107-107

60-62: LGTM!

Also applies to: 79-95, 322-322

@project516-review-bot project516-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR optimizes three hot paths: (1) PackIndex.children() builds a directory-to-names map once and caches it until the index grows, replacing per-listing scans of all paths. (2) JVM.slotOf uses nested maps (owner -> name -> slot) instead of concatenating a key string on every call. (3) fillStat writes time fields using literal names instead of building them in a loop. A shared TextDecoder is also reused in parsePack. All changes are localized and tests pass.


review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free, verdict approve

@Project516
Project516 merged commit 7786014 into master Nov 4, 2026
12 checks passed
@Project516
Project516 deleted the p516/index-children branch November 4, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant