-
Notifications
You must be signed in to change notification settings - Fork 393
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: shared module table at PluginContext #1144
base: main
Are you sure you want to change the base?
Conversation
f80d1d8
to
55d727d
Compare
✅ Deploy Preview for rolldown-rs canceled.
|
✅ Deploy Preview for rolldown-rs canceled.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1144 +/- ##
==========================================
- Coverage 86.95% 86.90% -0.05%
==========================================
Files 122 122
Lines 6018 6064 +46
==========================================
+ Hits 5233 5270 +37
- Misses 785 794 +9 ☔ View full report in Codecov by Sentry. |
Benchmarks Rust
|
CodSpeed Performance ReportMerging #1144 will not alter performanceComparing Summary
|
I give a rough look, it's not worth to wrap Compatibility not comes first. If we can't implement a compatible feature in reasonable way, we either try to find other ways or just give up the feature temperately. The compatible feature should not deeply affects the internal core code of rolldown |
I'm also agree it, I'm not sure for the compatibility, let us discuss it at the meeting. |
Description
The pr intend to support
PluginContext#getModuleInfo
, before i already supportNormalModule
cast toModuleInfo
, it only work atmoduleParesd
hook. But thePluginContext#getModuleInfo
need to valid at plugin lifetime. I consider two ways to support it.A. Add a
moduleInfos
(Dashmap) to ownerModuleInfo
atPluginContext
, but it need to set the module info when theModuleInfo
fields change, it need to re-set many times, due to https://rollupjs.org/plugin-development/#this-getmoduleinfo. It also has some overhead if the plugin not used it.B. Shared
ModuleTable
atPluginContext
, the typing isArc<RwLock<ModuleTable>>
, we only need to muate it at module loader and sort modules, it always do it at single thread, so we are usingRwLock
to avoid caused performance regression if lock conflict. Other, TheModuleInfo
will be lazy, if you used it, it will cast fromNormalModule
.The pr intend to implement
B
, it is sound good to me. If you have some good ideas, please let know.The mutate
SharedModuleTable
is not finished at module loader, it look like is complex, i intend to do it at next pr.