Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(config): reduce size by 350% using exact rule options #1876

Merged
merged 3 commits into from
Feb 21, 2024

Conversation

Conaclos
Copy link
Member

Summary

Part of #1263.
This should also help for #1795.

This PR reduces by 352% (from 4016 bytes to 1140 bytes) the memory usage of biome.json.
Most of the memory usage comes from the RuleConfiguration type (16 bytes) that is present for every rule.
RuleConfiguration is an enum with two variants, thus it took the size of its biggest variant: RuleWithOptions.
Most of the rules have no options, so this is just a waste of memory.

I managed to reduce the size of RuleWithOptions to only 1 byte for a rule without options.
This reduces the size of RuleConfiguration to 2 bytes :)
To do that, I added a type parameter to RuleConfiguration and RuleWithOptions.
This type parameter is set to the Options type of the rule. Most of the rule has Rule::Options set to ().
Rules with options took a variable amount of memory depending on the memory layout of their options.
I used the same trick as my previous PR (#1283) to ensure a maximum usage of 16 bytes: I wrapped the Options type in a Box when its size is larger than 16 (this is the case for 3 rules).

A nice side effect is that we get exact JSON schema.

We now generate an options.rs file in biome_js_analyze and biome_json_analyze that exports the options for the rules.
I had to make the rules public to satisfy the Rust compiler: it reported the use of a crate-scoped type in the public interface. Unfortunately disabling the Clippy check is not enough: the compiler still complained.

I also had to fix a bug in our deserializable derive macro that handled wrongly constrained generics.

Test Plan

CI should pass.

Copy link

netlify bot commented Feb 20, 2024

Deploy Preview for biomejs canceled.

Name Link
🔨 Latest commit 845a9d8
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/65d625b594df6e0008db139f

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project A-Linter Area: linter A-Tooling Area: internal tools L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS L-JSON Language: JSON and super languages labels Feb 20, 2024
Copy link
Contributor

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 49701 49701 0
Passed 48721 48721 0
Failed 980 980 0
Panics 0 0 0
Coverage 98.03% 98.03% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6322 6322 0
Passed 2036 2036 0
Failed 4286 4286 0
Panics 0 0 0
Coverage 32.20% 32.20% 0.00%

ts/babel

Test result main count This PR count Difference
Total 662 662 0
Passed 593 593 0
Failed 69 69 0
Panics 0 0 0
Coverage 89.58% 89.58% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 17646 17646 0
Passed 13439 13439 0
Failed 4205 4205 0
Panics 2 2 0
Coverage 76.16% 76.16% 0.00%

@Conaclos Conaclos force-pushed the conaclos/config/exact-options-type branch from c9920cc to 7fc9963 Compare February 20, 2024 21:14
@Conaclos Conaclos requested review from a team February 20, 2024 21:21
@Conaclos Conaclos force-pushed the conaclos/config/exact-options-type branch from 7fc9963 to ff29bef Compare February 20, 2024 21:32
Copy link

codspeed-hq bot commented Feb 20, 2024

CodSpeed Performance Report

Merging #1876 will not alter performance

Comparing conaclos/config/exact-options-type (845a9d8) with main (69f9031)

Summary

✅ 93 untouched benchmarks

@Conaclos Conaclos changed the title refactor(config): reduce size by 350% using exact config refactor(config): reduce size by 350% using exact rule options Feb 20, 2024
@Conaclos Conaclos force-pushed the conaclos/config/exact-options-type branch 4 times, most recently from ac190df to e057bc5 Compare February 20, 2024 23:02
Copy link
Contributor

@arendjr arendjr left a comment

Choose a reason for hiding this comment

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

Cool stuff!

@Conaclos
Copy link
Member Author

Conaclos commented Feb 21, 2024

I guess a trick like TypeId::of::() == TypeId::of::<()>() might work?

Good suggestion! Unfortunately this requires a 'static lifetime on every generic. I managed to avoid that by using std::mem::size_of_val(x) == 0 instead.

However, one snapshot (correct_root) is failing with an internal error (data did not match any variant of untagged enum RuleConfiguration). It seems that skipping the serialization of options affects in some way the desexualization.

I think it is fine to keep the current caveat because it is unlikely that someone uses level without options. Thus, serializing to { level: _, options: null } is unlikely.

@Conaclos Conaclos merged commit f8fccaa into main Feb 21, 2024
21 checks passed
@Conaclos Conaclos deleted the conaclos/config/exact-options-type branch February 21, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Linter Area: linter A-Project Area: project A-Tooling Area: internal tools L-CSS Language: CSS L-JavaScript Language: JavaScript and super languages L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants