Skip to content

feat/perf: more speed boost + logging improvements#3828

Merged
mrlubos merged 13 commits into
hey-api:mainfrom
SukkaW:improve-speed-2
Apr 28, 2026
Merged

feat/perf: more speed boost + logging improvements#3828
mrlubos merged 13 commits into
hey-api:mainfrom
SukkaW:improve-speed-2

Conversation

@SukkaW

@SukkaW SukkaW commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Logging Improvement

The logging now prints how many files were written, and the time usage in parse, render, and writing:

[Job 1] ✅ Done! Your output is in ./src/sdk (6 files in 6.5s)

Performance Improvement

Replace yaml with js-yaml in the YAML parser

@mrlubos In the end, I decided to make the switch still. For both Cloudflare (single huge YAML spec) and Redfish (a large quantity of ref YAML specs), I see a consistent 1+ second drop, from 11 seconds down to 9~10 seconds, so it is still a 9% improvements.

We don't actually ship @types/js-yaml to the user, it remains as a devDeps, we don't expose any YAML option in the first place.

Optimize walkDeclarations (single iteration)

Previously, the preferGroups path made multiple loops over all pointers (one per group to find matches, plus an unmatched pass). Replaced with a single loop that buckets each pointer directly into its group or unmatched[], then emits in order.

Replace collectPointerDependencies with a single iterative post-order DFS

The previous implementation called itself recursively from an outer loop, which is now replaced with a single iterative post-order DFS:

  • One shared inStack Set replaces N visited Set allocations.
  • childrenPushed marks first vs post-order visits on the explicit stack — no recursion.
  • Results are stored directly into graph.transitiveDependencies/graph.subtreeDependencies — no intermediate cache or double-write.
  • collectPointerDependencies, Cache, and PointerDependenciesResult types removed.

Pre-compute serialized index entries in SymbolRegistry

serializeIndexEntry (JSON.stringify) was called multiple times per entry across cacheKeyFromKeySpace, registerCacheDependencies, and invalidateCache. The serialized string is now computed once in buildIndexKeySpace and stored as entry[2], read as a plain field everywhere else. Also eliminated an intermediate string[] allocation in registerCacheDependencies by storing the IndexKeySpace reference directly in _cacheDependencies.

@bolt-new-by-stackblitz

Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@vercel

vercel Bot commented Apr 26, 2026

Copy link
Copy Markdown

@SukkaW is attempting to deploy a commit to the Hey API Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot

changeset-bot Bot commented Apr 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6e5249f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@hey-api/openapi-ts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pullfrog

pullfrog Bot commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

TL;DR — Broad performance sweep across the code generation pipeline: pre-computes serialized cache keys in the symbol registry, converts synchronous file I/O to parallel async writes, swaps the yaml parser for the faster js-yaml, and eliminates redundant loops and allocations in the graph walker, scope seeding, and declaration ordering. Also adds total elapsed time and file count to the CLI completion log.

Key changes

  • Pre-compute serialized index entries in SymbolRegistry — stores the serialized form directly as IndexEntry[2] at creation time, removing repeated serializeIndexEntry calls; inlines registerCacheDependencies into the query method for a single combined pass
  • Single-pass bucket sort in walkDeclarations — replaces the O(groups × pointers) pre-compute loop with a single pass that buckets pointers by group, using a Set for O(1) group membership checks
  • Inline seedLocalScopes into the graph-building loop — avoids a second full pass over graph.nodes by merging scope seeding with the parentToChildren cache build; replaces the /properties/[^/]+$/ regex with lastIndexOf-based string checks
  • Switch YAML parser from yaml to js-yaml — lighter dependency with faster parse times for the JSON Schema ref parser
  • Convert synchronous file writes to parallel asyncgenerateOutput now uses fsPromises with Promise.all for both openapi-ts and openapi-python
  • Add file count and total elapsed time to CLI output — the "Done!" log now shows file count (with correct singular/plural) and wall-clock duration via @lukeed/ms

Summary | 13 files | 13 commits | base: mainimprove-speed-2


Pre-computed serialization and inlined cache registration in SymbolRegistry

Before: serializeIndexEntry called on every cache lookup, invalidation, and dependency registration; registerCacheDependencies was a separate method with its own loop
After: serialized string stored as IndexEntry[2] at creation time; registerCacheDependencies removed and its logic folded into the query method's existing loop

The IndexEntry tuple gains a third element (serialized: string) populated once in buildIndexKeySpace. All consumers now read indexEntry[2] instead of calling serializeIndexEntry. The _cacheDependencies map stores the full IndexKeySpace directly (instead of a pre-serialized string array), and the reverse-index registration in _dependencyToCache happens inline during the query loop — combining index-set building with dependency tracking in a single pass.

registry.ts


Single-pass bucket sort in walkDeclarations

Before: two separate loops — one to pre-compute each pointer's group via matchPointerToGroup, then a nested loop O(groups × pointers) to emit in group order
After: single pass buckets pointers into a Map<string, string[]> by group; preferred groups are checked via a Set for O(1) membership; emission iterates buckets in order then the unmatched remainder

The fallback path (no preferGroups) also avoids materializing an intermediate array by iterating graph.nodes entries directly.

walk.ts


Merged scope seeding with parentToChildren cache build

Before: seedLocalScopes iterated all graph.nodes in a separate pass after the parentToChildren cache loop
After: scope seeding logic inlined into the same for loop that builds parentToChildren, eliminating a second full traversal

The regex pointer.match(/\/properties\/[^/]+$/) is replaced with two lastIndexOf calls and a slice comparison. instanceof Array checks in buildGraph are also normalized to Array.isArray().

graph.ts


Parallel async file writes and elapsed time logging

Before: files written sequentially with fs.writeFileSync / fs.mkdirSync; no timing or file count in CLI output
After: files written in parallel via fsPromises.mkdir + fsPromises.writeFile with Promise.all; CLI logs total file count and wall-clock duration with correct singular/plural handling

Both openapi-ts and openapi-python output generators now collect write promises and flush them concurrently. The generateOutput return type changes from Promise<void> to Promise<{ fileCount: number }>, and createClient captures total elapsed time via Date.now() to produce the new log line:

✅ Done! Your output is in ./output (42 files in 1.2s)

The @lukeed/ms package is added as a dependency to both openapi-ts and openapi-python for human-readable duration formatting. A changeset is included for the @hey-api/openapi-ts package.

output.ts (ts) · output.ts (py) · createClient.ts (ts) · createClient.ts (py)


yamljs-yaml in JSON Schema ref parser

Before: yaml package (heavier, spec-complete YAML 1.2 parser)
After: js-yaml (lighter, faster YAML parser sufficient for OpenAPI specs)

A straightforward dependency swap — parse from yaml replaced with load from js-yaml.

yaml.ts · package.json

Pullfrog  | View workflow run | via Pullfrog𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Solid set of optimizations. The registry pre-computation, iterative DFS, and walkDeclarations bucketing all look correct. A couple of items below worth addressing.

Task list (8/8 completed)
  • Read the PR diff and identify major areas of change
  • Read and review registry.ts changes
  • Read and review YAML parser + package.json changes
  • Read and review openapi-python changes
  • Read and review openapi-ts changes
  • Read and review shared/graph changes
  • Verify race condition risk in concurrent writes
  • Self-critique and submit review

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run𝕏

Comment thread packages/openapi-ts/src/createClient.ts Outdated
Comment thread packages/openapi-ts/src/generate/output.ts Outdated
Comment thread packages/openapi-python/src/generate/output.ts
Comment thread packages/shared/src/openApi/shared/utils/graph.ts Outdated
@pullfrog

pullfrog Bot commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Acknowledged the author's response on the parseStart placement comment — no code changes needed. The author is aware the timing includes I/O and considers it acceptable for now.

Task list (3/3 completed)
  • Read review thread and understand feedback
  • Reply to review comment acknowledging author's response
  • Report progress

Pullfrog  | View workflow run | via Pullfrog𝕏

@pullfrog

pullfrog Bot commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Acknowledged the author's reasoning on the "render" label — no code change needed. The label aptly distinguishes the in-memory output generation phase from the disk-write phase.

Task list (1/1 completed)
  • Reply to review comment about render label naming

Pullfrog  | View workflow run | via Pullfrog𝕏

Comment thread packages/openapi-ts/src/createClient.ts Outdated
@pullfrog

pullfrog Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Acknowledged the author's note that 46a033d makes the "render" label discussion moot. Replied and resolved the thread — no code changes needed.

Task list (3/3 completed)
  • Review the comment thread and determine action needed
  • Reply to the review comment acknowledging the update
  • Report progress

Pullfrog  | View workflow run | via Pullfrog𝕏

@pullfrog

pullfrog Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

No code changes needed — the parseStart placement concern was already addressed by the author in 46a033d. Replied and resolved the thread.

Task list (4/4 completed)
  • Read review comment and understand context
  • Determine if code changes are needed
  • Reply to review comment acknowledging the fix
  • Report progress

Pullfrog  | View workflow run | via Pullfrog𝕏

@pullfrog

pullfrog Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Acknowledged the author's reply — the mkdir optimization suggestion was already marked as non-blocking, and the author confirmed the total write time is negligible. No code changes needed.

Task list (3/3 completed)
  • Review the feedback from SukkaW on the mkdir optimization comment
  • Reply acknowledging the author's decision (no code change needed)
  • Report progress

Pullfrog  | View workflow run | via Pullfrog𝕏

@SukkaW SukkaW marked this pull request as ready for review April 27, 2026 02:55
@pullfrog

pullfrog Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Reviewed PR #3828 — no blocking issues found. All performance optimizations (iterative DFS, pre-computed index serialization, single-pass walk bucketing, yamljs-yaml swap, concurrent async writes) are correct. Review posted.

Task list (6/6 completed)
  • Read the PR diff and identify major areas of change
  • Review registry.ts changes (symbol registry)
  • Review graph walk and utils changes (shared package)
  • Review createClient and output changes (openapi-ts + openapi-python)
  • Review dependency changes (yaml parser, package.json, lock)
  • Self-critique and submit review

Pullfrog  | View workflow run | via Pullfrog𝕏

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. feature 🚀 Feature request. labels Apr 27, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed — no issues found.

Solid set of performance improvements with clean, well-commented implementations. The iterative post-order DFS replacing recursive collectPointerDependencies is the most substantial change and handles cycles correctly via the inStack guard. Pre-computing serialized index entries in IndexEntry[2] is a straightforward win that eliminates repeated JSON.stringify calls. The walkDeclarations single-pass bucketing and the regex-to-lastIndexOf replacement for scope seeding are both correct (the lastIndexOf approach is actually more precise than the old regex for pathological segment names).

Minor observation: openapi-ts/output.ts still uses sync fs.existsSync/fs.rmSync for the clean step (lines 16-17) while openapi-python/output.ts was fully converted to async fsPromises.access/fsPromises.rm. Not blocking — just an inconsistency to optionally align.

Task list (6/6 completed)
  • Read the PR diff and identify major areas of change
  • Review registry.ts changes (symbol registry)
  • Review graph walk and utils changes (shared package)
  • Review createClient and output changes (openapi-ts + openapi-python)
  • Review dependency changes (yaml parser, package.json, lock)
  • Self-critique and submit review

Pullfrog  | View workflow run𝕏

@codecov

codecov Bot commented Apr 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.20690% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.59%. Comparing base (460e71c) to head (6e5249f).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
packages/openapi-ts/src/generate/output.ts 54.54% 5 Missing ⚠️
packages/shared/src/graph/walk.ts 88.00% 3 Missing ⚠️
packages/shared/src/openApi/shared/utils/graph.ts 90.00% 2 Missing ⚠️
packages/codegen-core/src/symbols/registry.ts 96.15% 1 Missing ⚠️
packages/openapi-ts/src/createClient.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3828      +/-   ##
==========================================
+ Coverage   39.57%   39.59%   +0.02%     
==========================================
  Files         532      532              
  Lines       19544    19561      +17     
  Branches     5817     5825       +8     
==========================================
+ Hits         7734     7746      +12     
- Misses       9568     9573       +5     
  Partials     2242     2242              
Flag Coverage Δ
unittests 39.59% <86.20%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Apr 27, 2026

Copy link
Copy Markdown

Open in StackBlitz

@hey-api/codegen-core

npm i https://pkg.pr.new/@hey-api/codegen-core@3828

@hey-api/json-schema-ref-parser

npm i https://pkg.pr.new/@hey-api/json-schema-ref-parser@3828

@hey-api/nuxt

npm i https://pkg.pr.new/@hey-api/nuxt@3828

@hey-api/openapi-ts

npm i https://pkg.pr.new/@hey-api/openapi-ts@3828

@hey-api/shared

npm i https://pkg.pr.new/@hey-api/shared@3828

@hey-api/spec-types

npm i https://pkg.pr.new/@hey-api/spec-types@3828

@hey-api/types

npm i https://pkg.pr.new/@hey-api/types@3828

@hey-api/vite-plugin

npm i https://pkg.pr.new/@hey-api/vite-plugin@3828

commit: 6e5249f

@mrlubos

mrlubos commented Apr 28, 2026

Copy link
Copy Markdown
Member

@pullfrog how do you feel about the pull request now? Is everything resolved?

@pullfrog

pullfrog Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Yes — everything from my reviews is resolved. All four threads from the initial review were addressed (two via 46a033d, two accepted as-is with good rationale), and the incremental re-review found no new issues. The PR looks good to merge from my side.

Pullfrog  | View workflow run | via Pullfrog𝕏

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Apr 28, 2026
@mrlubos mrlubos merged commit 3c85ffb into hey-api:main Apr 28, 2026
10 of 11 checks passed
@hey-api hey-api Bot mentioned this pull request Apr 27, 2026
@SukkaW SukkaW deleted the improve-speed-2 branch May 3, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature 🚀 Feature request. lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants