HarperFast / HarperFast/harper
Skip the component-directory lock in symlinkHarperModule when the harper module links are already valid
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## What
Every non-root component load calls `symlinkHarperModule()`, which unconditionally goes through `Status.primaryStore.tryLock()` on the component directory — even when the `node_modules/harper` (and `node_modules/harperdb`) links are already correct, which is the overwhelmingly common case after the first worker has run.
On a multi-worker startup that means N workers × M app components all serialize on a per-directory cross-thread lock to perform a check that, for all but one of them, finds nothing to do.
[`components/componentLoader.ts:389`](https://github.com/HarperFast/harper/blob/main/components/componentLoader.ts#L389) (call site at [line 665](https://github.com/HarperFast/harper/blob/main/components/componentLoader.ts#L665)).
## Proposed change
Validate first, lock only to repair:
```ts
export function symlinkHarperModule(componentDirectory: string) {
if (harperModulesAreLinked(componentDirectory)) return Promise.resolve();
// ... existing tryLock / repair / unlock path
}
```
where `harperModulesAreLinked()` is a synchronous `lstatSync` + `realpathSync` check against `PACKAGE_ROOT`, and the repair body is extracted to a `repairHarperModuleLinks()` helper. This trades a small amount of synchronous metadata I/O per component for removal of the lock round-trip entirely on the steady-state path.
## Provenance
Written and reviewed as part of the now-closed #2343, which lost to #2314 on the shared startup-liveness fix (merged as `635fbd5387386cd4296ec56e25df8d68931da467`). The lock-free fast path is the one part of #2343 that #2314 does not contain, so it is filed here rather than left to die with the branch. The implementation exists on `fix/keep-workers-alive-until-ready` and can be lifted, but it should be rebuilt against post-#2314 `main` rather than cherry-picked — the surrounding function changed in #2314.
Two review conclusions from that branch, recorded so they are not re-derived:
- Comparing against bare `PACKAGE_ROOT` instead of `realpathSync(PACKAGE_ROOT)` is safe — it is already resolved at its definition in [`utility/packageUtils.js:47`](https://github.com/HarperFast/harper/blob/main/utility/packageUtils.js#L47). No need to pay a second `realpathSync` per check.
- Whether the waiter's lock timeout resolves or rejects is not load-bearing for startup — the sole call site already catches and logs, so neither outcome can fail a worker.
## Not in scope
The contended-repair path itself. A waiter that hits the 10s timeout still continues with the link in whatever state it is in, and a repair owner that dies mid-repair is still not retried by the waiter. #2314 made that path safe (bounded, non-fatal, no cross-thread unlock of a lock this thread never held), but it did not make it recoverable. Stale-owner reclamation needs its own design and is deliberately excluded here.
## Verification this needs
The claim is a contention reduction, and it is currently unmeasured — that is the main reason this is filed rather than merged. Whoever picks it up should land a before/after number for concurrent worker startup with several app components, not just a green test run. If the win doesn't show up, closing this is a fine outcome.
Contributor guide
Research direction
Start in components/componentLoader.ts at symlinkHarperModule() around line 389 and its call site near line 665; review utility/packageUtils.js:47 for PACKAGE_ROOT and the existing lock and repair flow. Rebuild the validation-first path against current main, then measure concurrent worker startup with several app components before and after. Done means the steady-state path avoids the lock and the contention reduction is quantified.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- nodejs, typescript
- Domain
- backend, performance
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 62/100