Skip to content

feat(sema): lower natspec#567

Open
0xrusowsky wants to merge 33 commits intoparadigmxyz:mainfrom
0xrusowsky:rusowsky/lower-natspec
Open

feat(sema): lower natspec#567
0xrusowsky wants to merge 33 commits intoparadigmxyz:mainfrom
0xrusowsky:rusowsky/lower-natspec

Conversation

@0xrusowsky
Copy link
Copy Markdown
Contributor

@0xrusowsky 0xrusowsky commented Oct 9, 2025

Motivation

towards full natspec support; continues the work started in:

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Oct 9, 2025

CodSpeed Performance Report

Merging #567 will degrade performances by 5.81%

Comparing 0xrusowsky:rusowsky/lower-natspec (906c10b) with main (6006669)

Summary

❌ 2 regressions
✅ 33 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
UniswapV3/lower 4.8 ms 5 ms -5.04%
Vm/lower 2.8 ms 2.9 ms -5.81%

@0xrusowsky
Copy link
Copy Markdown
Contributor Author

still wip, currently working on a rewrite to validete and resolve inheritdoc in a single pass

@0xrusowsky 0xrusowsky changed the title feat(hir): lower natspec feat(sema): lower natspec Oct 14, 2025
@0xrusowsky 0xrusowsky force-pushed the rusowsky/lower-natspec branch from 9f542dd to 557ce29 Compare October 14, 2025 18:04
@0xrusowsky 0xrusowsky marked this pull request as ready for review October 14, 2025 18:08
@0xrusowsky 0xrusowsky requested a review from onbjerg as a code owner October 14, 2025 18:08
@0xrusowsky
Copy link
Copy Markdown
Contributor Author

0xrusowsky commented Oct 14, 2025

@DaniPopes after validating all tags and resolving inheritdoc in a single pass, i managed to reduce the performance overhead on univ3/lower from ~20% to ~5%

unfortunately, that was useless on solady, as it doesn't have any @inheritdoc tags in the codebase... which means that what we want to optimize is the lowering/validation of the other tags.

i've tried several things like:

  • hinting the compiler which fns to inline + which should be cold
  • if also tried doing some lazy initialization for params/returns lookups
  • i was already sorting match arm variants according to (what i believe to be) hot paths, with @notice first, and @dev second.

but none of that proved to be useful, as solady is always at around 24% worse than without lowering :(

i acknowledge that solady has a bunch of comments, and that this must have some runtime overhead, but it kinda feels too much?

some things that i noticed, which maybe can help u come up with improvement ideas:

  • most of the comments are @dev comments (which don't require validation)
  • most of the comments are line comments, which means that:
    1. we don't group them
    2. multiline comments create new natspec items with the implicit @notice tag (don't require validation either)

lmk what else should i try to improve perf

@0xrusowsky 0xrusowsky requested a review from DaniPopes October 20, 2025 06:41
@DaniPopes
Copy link
Copy Markdown
Member

Please look at codspeed at what's slower. It's because it emits a bunch of diagnostics

@0xrusowsky
Copy link
Copy Markdown
Contributor Author

0xrusowsky commented Oct 29, 2025

i was under the impression that there could only be a single @author tag, but it turns out that solc allows multiple of them.

this was causing a bunch of emitted diagnostics in solady as u pointed out

perf regression went down to -5.8% on Vm.sol, solady is not an issue anymore, which makes much more sense!

thanks @DaniPopes

pub fn empty() -> &'ast Self {
// SAFETY: we're shortening the lifetime from 'static to 'ast for an empty, immutable slice,
// which is safe since the data never changes and lives forever.
unsafe { std::mem::transmute(Self::default()) }
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.

you don't need this if you implement Default for &'a <'b>

@onbjerg onbjerg added C-enhancement Category: an issue proposing an enhancement or a PR with one A-sema Area: semantic analysis labels Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-sema Area: semantic analysis C-enhancement Category: an issue proposing an enhancement or a PR with one

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants