Skip to content

perf: use TypedMap for CustomField and document plugin-context resolve rationale#8714

Merged
hyf0 merged 2 commits intoperf/replace-typed-dash-map-with-typed-mapfrom
copilot/sub-pr-8708
Mar 15, 2026
Merged

perf: use TypedMap for CustomField and document plugin-context resolve rationale#8714
hyf0 merged 2 commits intoperf/replace-typed-dash-map-with-typed-mapfrom
copilot/sub-pr-8708

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 15, 2026 •
edited
Loading

CustomField was using TypedDashMap even though plugin-context resolve treats it as write-once, then shared read-only state. This switches CustomField to TypedMap to remove unnecessary DashMap shard overhead and adds a short design note capturing when that choice should be revisited.

  • CustomField storage

    • Replace TypedDashMap with TypedMap in CustomField
    • Keep the existing usage model: populate locally, wrap in Arc, then read through resolve/load flows
    • Remove the typedmap dashmap feature from the dependency configuration

  • Call site adjustments

    • Update CustomField construction sites to insert values before sharing
    • Add DerefMut support so existing typed-map usage stays direct and local

  • Design note

    • Add meta/design/plugin-context-resolve.md
    • Record the current constraint: concurrent writes are not needed for PluginContextResolveOptions::custom
    • Document the future pivot point: switch back to TypedDashMap if plugin-context resolve starts requiring shared concurrent mutation

  • Code reference

    • Link CustomField back to the design note so the storage choice is easy to rediscover from the implementation
let mut custom = CustomField::default();
custom.insert(MyArg { id: 0 }, "hello, world".to_string());

let options = PluginContextResolveOptions {
  custom: Arc::new(custom),
  ..Default::default()
};

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI mentioned this pull request Mar 15, 2026
Merged
@hyf0
docs: add plugin-context-resolve design note
6f14dcb 
Co-authored-by: hyf0 <49502170+hyf0@users.noreply.github.com>
Copilot AI changed the title [WIP] Replace TypedDashMap with TypedMap in CustomField perf: use TypedMap for CustomField and document plugin-context resolve rationale Mar 15, 2026
Copilot AI requested a review from hyf0 March 15, 2026 15:37
Copilot finished work on behalf of hyf0 March 15, 2026 15:37
@hyf0 hyf0 marked this pull request as ready for review March 15, 2026 15:38
Copilot AI review requested due to automatic review settings March 15, 2026 15:38
@hyf0 hyf0 merged commit 564a2d8 into perf/replace-typed-dash-map-with-typed-map Mar 15, 2026
4 checks passed
@hyf0 hyf0 deleted the copilot/sub-pr-8708 branch March 15, 2026 15:39
Copilot AI reviewed Mar 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR documents and reinforces the design choice that PluginContextResolveOptions::custom is populated up-front and then shared read-only, aligning CustomField with a non-concurrent TypedMap-backed approach and capturing when that decision should be revisited.

Changes:

  • Add a design note describing why PluginContextResolveOptions::custom uses CustomField backed by TypedMap (not TypedDashMap).
  • Add an in-code reference from CustomField to the design note for discoverability.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
meta/design/plugin-context-resolve.md New design note documenting the CustomField backing rationale and future pivot point.
crates/rolldown_plugin/src/types/custom_field.rs Adds a pointer comment to the design note near the TypedMap backing type alias.

You can also share your feedback on Copilot code review. Take the survey.

crates/rolldown_plugin/src/types/custom_field.rs
Comment on lines +7 to 9
// See meta/design/plugin-context-resolve.md.
type Inner = TypedMap<(), typedmap::SyncAnyBounds, typedmap::SyncAnyBounds, FxBuildHasher>;

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 15, 2026

Benchmarks Rust

  • target: perf/replace-typed-dash-map-with-typed-map(367e721)
  • pr: copilot/sub-pr-8708(6f14dcb)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     64.9±1.42ms        ? ?/sec    1.01     65.7±1.94ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     73.2±1.85ms        ? ?/sec    1.00     72.9±1.19ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    140.0±3.21ms        ? ?/sec    1.03    144.2±3.28ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    160.3±3.99ms        ? ?/sec    1.01    161.8±3.06ms        ? ?/sec
bundle/bundle@threejs                                        1.00     61.1±2.26ms        ? ?/sec    1.00     61.2±2.79ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     72.8±2.95ms        ? ?/sec    1.00     72.6±3.04ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    705.4±5.99ms        ? ?/sec    1.00    703.8±4.53ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    812.9±4.67ms        ? ?/sec    1.00    809.4±5.48ms        ? ?/sec

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 15, 2026

Merging this PR will not alter performance

✅ 6 untouched benchmarks
⏩ 8 skipped benchmarks1

Comparing copilot/sub-pr-8708 (6f14dcb) with perf/replace-typed-dash-map-with-typed-map (367e721)2

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on perf/replace-typed-dash-map-with-typed-map (df3188d) during the generation of this report, so b578862 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@hyf0 hyf0 Awaiting requested review from hyf0

Assignees

@hyf0 hyf0

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hyf0