Feat: add a benchmark script for AgentLoader discovery - #902
Open
AmaadMartin wants to merge 5 commits into
Open
Conversation
added 5 commits
August 10, 2026 18:33
Performance work on the dev AgentLoader currently re-derives a throwaway timing harness per change, so the numbers are not comparable between changes. This script measures the real listAgents() call and splits it into three wall-clock segments: the work before the first esbuild build, the build window, and everything after the last build. It reads the built loader from dev/dist and leaves agent_loader.ts alone. It times the builds by swapping the esbuild module record for one whose build is wrapped, because esbuild exports build as a non-configurable getter. The script refuses to print numbers when that wrapper records no build, or when an agent fails to load.
Point contributors who change the loader's performance at the script, and state that it is manual and not part of CI.
Restore the discovery scan, the cache-busted import pass and the unattributed row. The import pass runs warm, because the loader has already imported the same bundles, so a new last row measures that import inside listAgents() and accounts for the residual. Also: report "nothing loaded" before "instrumentation detached", so an empty --dir no longer sends the reader to audit the loader; name a top-level agent.ts after the file, as loadAgentFromFile does; print --help before validating --runs; and claim an uncaught exception, because the loader answers one with an argument-less process.exit() that reports success.
AgentFile.load() unlinks the bundle of a file that compiled but exported no agent, before it throws the error the loader swallows. The import pass still imported that outfile, so a directory holding a helper file next to its agents crashed the benchmark instead of producing the report its own "counts differ" note describes. bundleReport already decides which bundles survived; the import pass now reads its list instead of the raw builds.
Drop the re-implemented entrypoint classification: the discovery row now times only the readdir/stat walk, and it runs after the timed call so it cannot warm the directory cache for it. The compiled count comes from the recorded builds, which is one per discovered entrypoint. Print both tables with console.table instead of hand-rolled column widths. Promote the loader's own import to a row of its own, so the unattributed residual explains itself and the caveat paragraph can go. Drop WARMUP_RUNS, a constant nobody sets driving a loop that ran once. CONTRIBUTING now points at --help, which carries the preconditions, instead of repeating the commands.
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
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.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
N/A
Problem: Several changes to
dev/src/utils/agent_loader.tsclaim a performance win, and each one builds its own throwaway timing harness. The numbers are therefore not comparable between changes, and they are easy to overstate.Solution: Add one committed, dependency-free script that everybody runs the same way.
scripts/bench_agent_discovery.mjstimes the discovery scan, the realesbuild.build()calls, anawait import()pass over the compiled bundles and the end-to-endlistAgents()call, then reports the minimum and the median over N runs plus the compiled size of each agent bundle. It does not modify the loader, it adds no dependency, and it is not wired into CI or into any vitest project.Collision check:
gh pr list --repo AmaadMartin/adk-js --state all --limit 1000matched no open or closed pull request for "bench". The adjacent loader pull requests (#814, #633, #858, #506) all change the loader; none of them adds a benchmark. This branch does not overlap them and does not stack on any of them.Design notes:
esbuildbefore the loader imports it. esbuild exportsbuildas a non-configurable getter, so assignment andObject.definePropertyboth fail on the real module object.loader importandimportmeasure the same work twice on purpose. The import pass runs after the loader has already imported the same four near-identical 5.7 MiB bundles, so V8 answers it from a warm compilation cache and reads 556 ms against the loader's own 1523 ms. The pair explains theunattributedresidual, andloader importis the number to quote for import cost.readdir/statwalk, and it runs after the timed call so that it cannot warm the directory cache for it. The compiled count comes from the recorded builds rather than from a second copy of the loader's entrypoint rules.esbuild.buildcall means the instrumentation detached, and a recorded load failure means the run measured a directory that did not load.No automated test is added, deliberately. The script is a manual tool outside the vitest projects, and a test that exercised it meaningfully would compile four 5.7 MiB bundles for an assertion that a number was printed.
scripts/**is outside the vitest coverageincludelist, so the coverage thresholds do not move.Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[ ] I have added or updated unit tests for my change. — none added, deliberately; see the reasoning above.
[x] All unit tests pass locally.
npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts -t "AgentLoader discovery"— 3 passed. This suite covers the loader behaviour the script measures, and the script does not change the loader, the fixture orvitest.config.ts.The sibling suite in that file,
App entrypoint with app_ts | app_js | app_default, fails in my sandbox before my change and after it. ItsbeforeAllrunsnpm installunder a 60 s hook timeout and my proxy needs about 120 s; with the fixture pre-installed the remaining failure is the spawned CLI, which needs model credentials I do not have. Neither failure touches this change.Repository gates:
npm run lint,npm run format:check,npm run docs:check,npx secretlint "scripts/**"andbash scripts/check_license.shall pass.npm run ts:checkfails, but it fails identically onmain(errors incore/test/**), andtsc --listFilesOnlyconfirms it never reads the new.mjsfile.Fork CI:
run-testsfails on this branch with7 failed | 290 passed | 20 skipped. The validation run onmainat 09e2375, which is this branch's merge base, fails with the identical counts and the identical files, all undertests/integration/workflows/. This branch adds no TypeScript, so it cannot reach them.Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.
Output on Node v22.22.2, linux/x64, 64 cpus (the repository path is redacted):
Import dominates compile: 1523 ms of loader import against a 1112 ms build window.
Proof that the measurement is real. Three mutations, each reverted afterwards.
delete require.cache[...]from the import pass collapsed theimportrow from 565.4 ms to 37.6 ms (median). Without it a cache-bustedimport()of a.cjsbundle resolves from the CommonJS cache and re-executes nothing.esbuild.build(...), injected into the built loader underdev/distand never into the source, raised the pre-build time by 500 ms and end-to-end to 3094.6 ms.await import(importUrl)raised theloader importrow from 1533.0 ms to 2032.1 ms (median) and end-to-end to 3105.7 ms.Cannot find module '/tmp/adk_agent_loader-*/types.cjs'. That is the shape the fix pins.Guard and error paths.
the esbuild instrumentation did not attach: no esbuild.build call was recorded.no agent loaded from <dir>: nothing to measure.3 compiled, 1 loadedand a note; the deleted bundles are dropped from both the size table and the import passdev/distmissingrun 'npm run build' first, no stack trace--dir ./nope--help --runs 0--helpis answered before--runsis validatedagent.tsagent, matchingloadAgentFromFile--runs 0,--runs abc,--bogus--runs 1,--runs 4,--runs 7/tmp/adk_agent_loader-*is unchangedChecklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[ ] I have added tests that prove my fix is effective or that my feature works. — proven by the mutation runs above instead.
[x] New and existing unit tests pass locally with my changes.