Skip to content

Comments

refactor: use less memory to store frequently accessed options field#5948

Merged
IWANABETHATGUY merged 1 commit intomainfrom
08-28-refactor_use_less_memory_to_store_frequently_accessed_options_field
Aug 28, 2025
Merged

refactor: use less memory to store frequently accessed options field#5948
IWANABETHATGUY merged 1 commit intomainfrom
08-28-refactor_use_less_memory_to_store_frequently_accessed_options_field

Conversation

@IWANABETHATGUY
Copy link
Member

@IWANABETHATGUY IWANABETHATGUY commented Aug 28, 2025

TL;DR

Refactored the SideEffectDetector to use bitflags for improved performance and cleaner code.

What changed?

  • Added bitflags dependency and created a new SideEffectDetectorFlatOptions struct to replace individual boolean fields
  • Consolidated multiple boolean fields (ignore_annotations, jsx_preserve, is_manual_pure_functions_empty) into a single bitflags field
  • Added a new flag for property_read_side_effects to cache the result of the property read side effects check
  • Created inline accessor methods for each flag to maintain code readability
  • Replaced direct field access with these accessor methods throughout the code

How to test?

  • Run the existing test suite to ensure the refactoring doesn't change behavior
  • Verify that side effect detection works correctly with various combinations of options
  • Check that property read side effects are properly detected based on configuration

Why make this change?

This refactoring improves performance by:

  1. Reducing memory usage by storing multiple boolean flags in a single byte
  2. Caching the result of property read side effects check rather than recomputing it
  3. Making the code more maintainable with clear accessor methods for each flag

The change also makes the code more maintainable by centralizing option handling and providing a clearer interface for accessing configuration values.

Copy link
Member Author

IWANABETHATGUY commented Aug 28, 2025

@github-actions
Copy link
Contributor

github-actions bot commented Aug 28, 2025

Benchmarks Rust

  • target: main(037ac70)
  • pr: 08-28-refactor_use_less_memory_to_store_frequently_accessed_options_field(4c31278)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.03     82.6±2.68ms        ? ?/sec    1.00     79.9±1.73ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.03     92.8±2.50ms        ? ?/sec    1.00     90.1±1.16ms        ? ?/sec
bundle/bundle@rome_ts                                        1.03    124.3±4.30ms        ? ?/sec    1.00    121.0±1.92ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    141.9±3.23ms        ? ?/sec    1.00    141.8±2.36ms        ? ?/sec
bundle/bundle@threejs                                        1.01     44.9±1.39ms        ? ?/sec    1.00     44.4±2.51ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.05     55.9±1.55ms        ? ?/sec    1.00     53.0±0.83ms        ? ?/sec
bundle/bundle@threejs10x                                     1.02    478.3±5.88ms        ? ?/sec    1.00    468.8±6.19ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    557.2±6.82ms        ? ?/sec    1.00    554.4±7.66ms        ? ?/sec
scan/scan@rome_ts                                            1.00     94.3±1.63ms        ? ?/sec    1.03     97.5±2.47ms        ? ?/sec
scan/scan@threejs                                            1.00     33.3±1.48ms        ? ?/sec    1.00     33.4±1.06ms        ? ?/sec
scan/scan@threejs10x                                         1.00    349.8±4.56ms        ? ?/sec    1.01    354.0±5.56ms        ? ?/sec

@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review August 28, 2025 08:21
@IWANABETHATGUY IWANABETHATGUY force-pushed the 08-28-refactor_use_less_memory_to_store_frequently_accessed_options_field branch 2 times, most recently from 0f328ed to 8f8fbdd Compare August 28, 2025 08:59
@IWANABETHATGUY IWANABETHATGUY force-pushed the 08-27-feat_5872 branch 2 times, most recently from 287f3a0 to 7e7b795 Compare August 28, 2025 09:15
@IWANABETHATGUY IWANABETHATGUY force-pushed the 08-28-refactor_use_less_memory_to_store_frequently_accessed_options_field branch from 8f8fbdd to 27ce45a Compare August 28, 2025 09:15
Base automatically changed from 08-27-feat_5872 to main August 28, 2025 09:36
@IWANABETHATGUY IWANABETHATGUY force-pushed the 08-28-refactor_use_less_memory_to_store_frequently_accessed_options_field branch from 27ce45a to 4c31278 Compare August 28, 2025 09:36
@graphite-app
Copy link
Contributor

graphite-app bot commented Aug 28, 2025

Merge activity

  • Aug 28, 9:36 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@netlify
Copy link

netlify bot commented Aug 28, 2025

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 4c31278
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/68b023159de5ba00089968b9

@IWANABETHATGUY IWANABETHATGUY added this pull request to the merge queue Aug 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 28, 2025
@IWANABETHATGUY IWANABETHATGUY merged commit 09edd36 into main Aug 28, 2025
22 checks passed
@IWANABETHATGUY IWANABETHATGUY deleted the 08-28-refactor_use_less_memory_to_store_frequently_accessed_options_field branch August 28, 2025 10:02
@github-actions github-actions bot mentioned this pull request Sep 5, 2025
shulaoda added a commit that referenced this pull request Sep 5, 2025
## [1.0.0-beta.35] - 2025-09-05

### 🚀 Features

- rolldown_plugin_vite_css_post: attch vite metadata to chunks (#6011) by @shulaoda
- rolldown_plugin_vite_css_post: emit a single CSS asset for non–code-split builds (#6005) by @shulaoda
- rolldown_plugin_vite_css_post: align empty CSS chunk removal logic (#6004) by @shulaoda
- use frequent characters first for internal export names (#5524) by @AliceLanniste
- rolldown_plugin_vite_css_post: align partial `generateBundle` logic (#5987) by @shulaoda
- rolldown_plugin_vite_css_post: align `augmentChunkHash` logic (#5986) by @shulaoda
- rolldown_watcher: introduce `WatcherConfig` for configurable watcher parameters (#5991) by @hyf0
- dev: support `import.meta.invalidate` and migrate tests (#5979) by @hyf0
- dev: adapt `TestDevServer` with `DevEngine` (#5976) by @hyf0
- propertyWriteSideEffects (#5977) by @IWANABETHATGUY
- rolldown_plugin_vite_css_post: complete `transform` logic (#5985) by @shulaoda
- dev: generate hmr updates for file changes (#5961) by @hyf0
- rolldown_plugin_vite_css_post: complete `finalize_css` (#5974) by @shulaoda
- dev: manage cache by `DevEngine` (#5960) by @hyf0
- rolldown_plugin_vite_css_post: align `hoist_at_rules` (#5967) by @shulaoda
- rolldown_plugin_vte_css_post: complete `resolve_asset_urls_in_css` (#5958) by @shulaoda
- rolldown_plugin_utils: support common `to_output_file_path` (#5956) by @shulaoda
- dev: default to not eager rebuild (#5949) by @hyf0
- treeshake.propertyReadSideEffects (#5945) by @IWANABETHATGUY
- improve error message for `this.resolve` and `this.load` (#5596) by @sapphi-red
- dev: accept `onHmrUpdates` callback (#5942) by @hyf0
- rolldown_plugin_vite_css_post: align partial `resolve_asset_urls_in_css` (#5929) by @shulaoda
- mark `__export` runtime helper as pure (#5926) by @IWANABETHATGUY
- rolldown_plugin_vite_css_post: extract `finalize_css_chunk` (#5916) by @shulaoda
- implement inlineConst.pass (#5912) by @IWANABETHATGUY
- rolldown_plugin_vite_css_post: align partial legacy logic (#5915) by @shulaoda
- add inlineConst.pass options  (#5911) by @IWANABETHATGUY
- rolldown_plugin_vite_css_post: align partial css code split logic (#5906) by @shulaoda
- inlineConst: safe (#5899) by @IWANABETHATGUY

### 🐛 Bug Fixes

- rolldown: don't cleanup for browser build (#6024) by @sxzz
- propertyWriteSideEffects for toplevel staticClassBlock (#5989) by @IWANABETHATGUY
- handle `obj().prop` when `propertyReadSideEffects: false` (#5988) by @IWANABETHATGUY
- handle objectSpread when `treeshake.propertyReadSideEffects` is enabled (#5981) by @IWANABETHATGUY
- __toESM function breaking ES module imports (#5966) by @IWANABETHATGUY
- merge `typescript.onlyRemoveTypeImports` correctly (#5962) by @shulaoda
- should not generate `init_mod` when record is a ExportAllDeclaration and importee is a inner concatenate module (#5952) by @IWANABETHATGUY
- use symbol existance to detect if a plugin is BuiltinPlugin (#5940) by @IWANABETHATGUY
- handle errors thrown in `onLog` and `onwarn` options (#5931) by @sapphi-red
- `replace_plugin` does not work as expected with .ts config (#5920) by @IWANABETHATGUY
- `replace_plugin` support primitive values replacement  (#5921) by @IWANABETHATGUY
- node 20 test version (#5918) by @IWANABETHATGUY
- trigger trace subscriber cleanup on Node.js side (#5913) by @sapphi-red
- add friendly deprecation warning for `resolve.tsconfigFilename` (#5908) by @shulaoda
- inlineConst inlines a var (#5903) by @IWANABETHATGUY
- types: omit `sourcemap` property from `MinifyOptions` correctly (#5892) by @sapphi-red

### 🚜 Refactor

- hmr: process changed files in one update (#6013) by @hyf0
- rolldown_plugin_vite_css_post: improve (#6006) by @shulaoda
- migrate remaining crates from #[allow] to #[expect] attributes (#6002) by @hyf0
- crates/rolldown_common: migrate from #[allow] to #[expect] attributes (#6001) by @hyf0
- crates/rolldown_binding: migrate from #[allow] to #[expect] attributes (#6000) by @hyf0
- crates/rolldown: migrate from #[allow] to #[expect] attributes (#5999) by @hyf0
- extract all options usage in `impl_visit.rs` into `FlatOptionsFlag` (#5992) by @IWANABETHATGUY
- rolldown_watcher: distinguish debounced and non-debounced watchers (#5990) by @hyf0
- dev: only use poll-based watch if required (#5984) by @hyf0
- dev: use dynamic dispatch watcher (#5982) by @hyf0
- improve plugin logic relate to `to_output_file_path` (#5959) by @shulaoda
- make reference_needed_symbols lock free (#5964) by @IWANABETHATGUY
- tweak module loader code (#5950) by @shulaoda
- use less memory to store frequently accessed options field (#5948) by @IWANABETHATGUY
- make `ecma_related` in `NormalModuleTaskResult` non-optional (#5947) by @shulaoda
- dev: replace `BuildStatus` with `BuildStateMachine` (#5927) by @hyf0
- rename `inlineConst: 'safe'` to `inlineConst: 'smart'` (#5909) by @IWANABETHATGUY

### 📚 Documentation

- add redirect for old plugin development page link (#5963) by @TheAlexLichter

### ⚡ Performance

- rolldown: use allocator pool when minifying chunks (#5978) by @Boshen
- merge two `PreProcessor` and `EnsureSpanUniqueness` (#5968) by @IWANABETHATGUY

### 🧪 Testing

- cjs module lexer for named import external with cjs format (#5970) by @IWANABETHATGUY
- hmr: import.meta.hot?.accept case (#5935) by @sapphi-red
- hmr: accept outside circular case (#5938) by @sapphi-red
- hmr: no accept outside circular dependencies case (#5937) by @sapphi-red
- hmr: self accept within circular dependencies case (#5936) by @sapphi-red

### ⚙️ Miscellaneous Tasks

- deps: update crate-ci/typos action to v1.36.2 (#6015) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to ^0.16.0 (#6023) by @renovate[bot]
- ci: re-enable WASM tests in CI workflows (#6007) by @hyf0
- deps: update crate-ci/typos action to v1.35.8 (#6012) by @renovate[bot]
- clippy: enable allow_attributes lint and migrate to #[expect] (#6008) by @hyf0
- remove unmaintained AGENTS.md file (#6009) by @hyf0
- deps: update github-actions (#5993) by @renovate[bot]
- fix `knip` warnings and remove redundant `@rolldown/testing` (#5973) by @shulaoda
- deps: update dependency rolldown-plugin-dts to v0.15.10 (#5925) by @renovate[bot]
- deps: update rust crate tracing-subscriber to v0.3.20 [security] (#5972) by @renovate[bot]
- fix wasi build failed (#5969) by @shulaoda
- deps: update crate-ci/typos action to v1.35.7 (#5965) by @renovate[bot]
- remove redundant code (#5943) by @shulaoda
- npm trusted publisher (#5953) by @Brooooooklyn
- deps: update crate-ci/typos action to v1.35.6 (#5957) by @renovate[bot]
- Revert "feat: mark `__export` runtime helper as pure (#5926)" (#5928) by @IWANABETHATGUY
- deps: update dependency rolldown-plugin-dts to v0.15.10 (#5900) by @renovate[bot]
- validator: improve the diagnostic message (#5919) by @shulaoda
- add more tracing instrumentation (#5914) by @sapphi-red
- add proper error message when passing unexpected minify options in rust test (#5905) by @IWANABETHATGUY
- deps: update dependency tsdown to v0.14.2 (#5901) by @renovate[bot]
- add automatic issue closing (#5895) by @shulaoda

Co-authored-by: shulaoda <[email protected]>
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.

2 participants