Skip to content

impr: Skip uniform alignment analysis when its warning is off - #3234

Closed
iwoplaza wants to merge 1 commit into
software-mansion:mainfrom
iwoplaza:impr/skip-disabled-alignment-warning
Closed

iwoplaza wants to merge 1 commit into
software-mansion:mainfrom
iwoplaza:impr/skip-disabled-alignment-warning

Conversation

@iwoplaza

Copy link
Copy Markdown
Collaborator

Every $usage('uniform') walked the schema and computed memory layouts to decide whether to warn about non-portable uniform alignment (uniform-schema-misaligned). That warning is off by default in production builds, so the work was wasted there.

  • Adds isEnabled(type) to the internal logger, so callers can skip computing what a warning needs when it won't be shown.
  • warnIfNotUniformAligned returns early when its warning is disabled. In development and tests nothing changes.

Node 22 with NODE_ENV=production, creating root.createBuffer(Scene).$usage('uniform') for a scene struct (camera matrices plus an array of 16 light structs). Medians of 9 fresh processes:

main this PR
First buffer 1.90 ms 0.45 ms
Each of the next 200 69.2 µs 3.8 µs

Every `$usage('uniform')` walked the schema and computed memory layouts to decide whether to warn about non-portable uniform alignment (`uniform-schema-misaligned`). That warning is off by default in production builds, so the work was wasted there.

- Adds `isEnabled(type)` to the internal logger, so callers can skip computing what a warning needs when it won't be shown.
- `warnIfNotUniformAligned` returns early when its warning is disabled. In development and tests nothing changes.

Node 22 with `NODE_ENV=production`, creating `root.createBuffer(Scene).$usage('uniform')` for a scene struct (camera matrices plus an array of 16 light structs). Medians of 9 fresh processes:

| | main | this PR |
|---|---|---|
| First buffer | 1.90 ms | 0.45 ms |
| Each of the next 200 | 69.2 µs | 3.8 µs |

Copilot AI 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.

🟢 Approval recommended

The narrow guard matches the logger’s existing warning state and preserves enabled-warning behavior, with no unresolved findings.

No open issues 🎉

What changed in this PR

Avoids unnecessary uniform-alignment analysis when its warning is disabled, reducing buffer creation overhead in production.

Changes:

  • Adds an internal logger method to query warning enablement.
  • Skips alignment analysis when disabled, preserving existing behavior when enabled.
File Description
packages/​typegpu/​src/​tgpuLogger.ts Adds isEnabled() using existing warning state.
packages/​typegpu/​src/​core/​pipeline/​webgpuLimitations.ts Returns before alignment analysis when the warning is disabled.

Review effort: Balanced

@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.

✅ No new issues found.

Reviewed changes

  • isEnabled(type) added to the internal logger. TgpuLogger.isEnabled mirrors the #enabledWarnings.has(type) guard already used by warn/warnOnce; the method is only reachable through the unexported internal Logger object (logger), so no public API surface changes.
  • Early return in warnIfNotUniformAligned. The alignment walk now exits before computing strides/layouts when uniform-schema-misaligned is disabled. In DEV and tests all warnings are enabled, so the behavior and the tests/buffer.test.ts "Uniform alignment" snapshots are unchanged; in production the warning stays off and the analysis is skipped as intended.

The function has no side effects beyond warnings and TgpuLogger is the repo's only Logger implementation, so the added interface member has no other implementer to update.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment on lines +50 to +53
if (!logger.isEnabled('uniform-schema-misaligned')) {
// Silenced by default in production, and computing memory layouts isn't free
return;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

very nit: If this check does not return, it is run for each of structs' props (this function is recursive)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That's true, though the check is most likely very cheap, and only runs in dev mode anyway.

@iwoplaza

Copy link
Copy Markdown
Collaborator Author

Moved to #3238 (branch now lives on the main repo instead of the fork).

@iwoplaza iwoplaza closed this Oct 21, 2026
@iwoplaza
iwoplaza deleted the impr/skip-disabled-alignment-warning branch October 21, 2026 09:39
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.

3 participants