perf: improve FlagDependencyExportsPlugin for large JSON by depth #8802
Conversation
|
|
github-actions
bot
added
the
release: performance
✅ Deploy Preview for rspack canceled.Built without sensitive environment variables
|
hai-x
force-pushed
the
hai-x/feat-support-dyn-exp-for-url
branch
from
84ce1e2 to
cad432f
Compare
CodSpeed Performance ReportMerging #8802 will not alter performanceComparing 🎉 Hooray!
|
| @@ -13,13 +13,15 @@ pub struct JsonExportsDependency { | |||
| id: DependencyId, | |||
| #[cacheable(with=AsPreset)] | |||
| data: JsonValue, | |||
| exports_depth: f64, | |||
There was a problem hiding this comment.
| exports_depth: f64, | |
| exports_depth: u32, |
u32 should be enough
crates/rspack_plugin_json/src/lib.rs
Outdated
| @@ -32,7 +32,9 @@ mod utils; | |||
|
|
|||
| #[cacheable] | |||
| #[derive(Debug)] | |||
| struct JsonParserAndGenerator; | |||
| struct JsonParserAndGenerator { | |||
| pub exports_depth: f64, | |||
There was a problem hiding this comment.
| pub exports_depth: f64, | |
| pub exports_depth: u32, |
| D( | ||
| module.parser[JSON_MODULE_TYPE], | ||
| "exportsDepth", | ||
| mode === "development" ? 1 : Number.POSITIVE_INFINITY |
There was a problem hiding this comment.
| mode === "development" ? 1 : Number.POSITIVE_INFINITY | |
| mode === "development" ? 1 : Number.MAX_SAFE_INTEGER |
There was a problem hiding this comment.
It is Infinity in webpack. Should we align?
There was a problem hiding this comment.
I think both is fine, but I'm not sure what will pass to rust side by napi-rs if we use Infinity here
There was a problem hiding this comment.
Make sense. After tested napi locally. I Found that:
js pass Infinity & rust declare f64 => exportsDepth: f64::INFINITY(rust side)
js pass Infinity & rust declare u32 => exportsDepth: 0
js pass MAX_SAFE_INTEGER & rust declare f64 => exportsDepth: 9007199254740991 (2^53-1)
js pass MAX_SAFE_INTEGER & rust declare u32 => exportsDepth: 4294967295 (2^32-1)
Should we keep f64 + Infinity to align?
There was a problem hiding this comment.
Both fine to me, but I prefer u32 a little bit :)
There was a problem hiding this comment.
Both fine to me, but I prefer
u32a little bit :)
No problem. Let we use u32. I think it's more efficient in rust side.
When there is feedback from users, we can make further changes.
I will update it later.
hai-x
force-pushed
the
hai-x/feat-support-dyn-exp-for-url
branch
from
bb05fe4 to
610aaf0
Compare
Summary
Aligned with webpack/webpack#19058.
I will update doc later.
Checklist