Skip to content

Commit

Permalink
perf: reduce allocation for parsing (#7219)
Browse files Browse the repository at this point in the history 
perf: init
  • Loading branch information
@h-a-n-a
h-a-n-a authored Jul 19, 2024
1 parent 75150e3 commit 88c6d6a
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 46 deletions.
19 changes: 18 additions & 1 deletion crates/rspack_identifier/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,25 @@ impl From<Identifier> for Ustr {
}
}

impl Identifier {
/// Convert [Identifier] to [String]
///
/// Shadowed the [fmt::Display] to specialize `to_string`,
/// like how other structs are shadowed in the standard library.
/// See: https://github.com/rust-lang/rust/pull/32586
///
/// Consistency:
/// The result of `to_string` should be the same as the result of [fmt::Display::fmt].
#[allow(clippy::inherent_to_string_shadow_display)]
pub fn to_string(&self) -> String {
self.0.to_owned()
}
}

impl fmt::Display for Identifier {
/// Consistency:
/// The result of `to_string` should be the same as the result of [fmt::Display::fmt].
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0.as_str())
write!(f, "{}", self.to_string())
}
}
2 changes: 1 addition & 1 deletion crates/rspack_plugin_javascript/src/parser_plugin/compatibility_plugin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl JavascriptParserPlugin for CompatibilityPlugin {
}
let tag_info = parser
.definitions_db
.expect_get_mut_tag_info(&parser.current_tag_info?);
.expect_get_mut_tag_info(parser.current_tag_info?);

let mut nested_require_data = NestedRequireData::downcast(tag_info.data.take()?);
let mut deps = Vec::with_capacity(2);
Expand Down
6 changes: 3 additions & 3 deletions crates/rspack_plugin_javascript/src/parser_plugin/harmony_import_dependency_parser_plugin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl JavascriptParserPlugin for HarmonyImportDependencyParserPlugin {
}
let tag_info = parser
.definitions_db
.expect_get_tag_info(&parser.current_tag_info?);
.expect_get_tag_info(parser.current_tag_info?);
let settings = HarmonySpecifierData::downcast(tag_info.data.clone()?);

let dep = HarmonyImportSpecifierDependency::new(
Expand Down Expand Up @@ -178,7 +178,7 @@ impl JavascriptParserPlugin for HarmonyImportDependencyParserPlugin {
}
let tag_info = parser
.definitions_db
.expect_get_tag_info(&parser.current_tag_info?);
.expect_get_tag_info(parser.current_tag_info?);
let settings = HarmonySpecifierData::downcast(tag_info.data.clone()?);

let non_optional_members = get_non_optional_part(members, members_optionals);
Expand Down Expand Up @@ -243,7 +243,7 @@ impl JavascriptParserPlugin for HarmonyImportDependencyParserPlugin {
}
let tag_info = parser
.definitions_db
.expect_get_tag_info(&parser.current_tag_info?);
.expect_get_tag_info(parser.current_tag_info?);
let settings = HarmonySpecifierData::downcast(tag_info.data.clone()?);

let non_optional_members = get_non_optional_part(members, members_optionals);
Expand Down
4 changes: 2 additions & 2 deletions crates/rspack_plugin_javascript/src/parser_plugin/inner_graph/plugin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl InnerGraphPlugin {
}

if let Some(tag_info) = parser.current_tag_info {
let tag_info = parser.definitions_db.expect_get_tag_info(&tag_info);
let tag_info = parser.definitions_db.expect_get_tag_info(tag_info);
let symbol = TopLevelSymbol::downcast(tag_info.data.clone().expect("should have data"));
let usage = parser.inner_graph.get_top_level_symbol();
parser.inner_graph.add_usage(
Expand Down Expand Up @@ -295,7 +295,7 @@ impl InnerGraphPlugin {
let existing = parser.get_variable_info(name);
if let Some(existing) = existing
&& let Some(tag_info) = existing.tag_info
&& let tag_info = parser.definitions_db.expect_get_mut_tag_info(&tag_info)
&& let tag_info = parser.definitions_db.expect_get_mut_tag_info(tag_info)
&& tag_info.tag == TOP_LEVEL_SYMBOL
&& let Some(tag_data) = tag_info.data.clone()
{
Expand Down
6 changes: 3 additions & 3 deletions crates/rspack_plugin_javascript/src/parser_plugin/worker_plugin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ impl JavascriptParserPlugin for WorkerPlugin {
}
let tag_info = parser
.definitions_db
.expect_get_tag_info(&parser.current_tag_info?);
.expect_get_tag_info(parser.current_tag_info?);
let data = WorkerSpecifierData::downcast(tag_info.data.clone()?);
if let Some(value) = self.pattern_syntax.get(data.key.as_str())
&& value.contains(&members.iter().map(|id| id.as_str()).join("."))
Expand All @@ -331,7 +331,7 @@ impl JavascriptParserPlugin for WorkerPlugin {
if for_name == HARMONY_SPECIFIER_TAG {
let tag_info = parser
.definitions_db
.expect_get_tag_info(&parser.current_tag_info?);
.expect_get_tag_info(parser.current_tag_info?);
let settings = HarmonySpecifierData::downcast(tag_info.data.clone()?);
let ids = settings.ids.iter().map(|id| id.as_str()).join(".");
if self
Expand Down Expand Up @@ -371,7 +371,7 @@ impl JavascriptParserPlugin for WorkerPlugin {
if for_name == HARMONY_SPECIFIER_TAG {
let tag_info = parser
.definitions_db
.expect_get_tag_info(&parser.current_tag_info?);
.expect_get_tag_info(parser.current_tag_info?);
let settings = HarmonySpecifierData::downcast(tag_info.data.clone()?);
let ids = settings.ids.iter().map(|id| id.as_str()).join(".");
if self
Expand Down
6 changes: 3 additions & 3 deletions crates/rspack_plugin_javascript/src/visitors/dependency/parser/call_hooks_name.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,12 @@ fn call_hooks_info<F, T>(
where
F: Fn(&mut JavascriptParser, &str) -> Option<T>,
{
let info = parser.definitions_db.expect_get_variable(&id);
let info = parser.definitions_db.expect_get_variable(id);
let mut next_tag_info = info.tag_info;

while let Some(tag_info_id) = next_tag_info {
parser.current_tag_info = Some(tag_info_id);
let tag_info = parser.definitions_db.expect_get_tag_info(&tag_info_id);
let tag_info = parser.definitions_db.expect_get_tag_info(tag_info_id);
let tag = tag_info.tag.to_string();
let next = tag_info.next;
let result = hook_call(parser, &tag);
Expand All @@ -134,7 +134,7 @@ where
next_tag_info = next;
}

let info = parser.definitions_db.expect_get_variable(&id);
let info = parser.definitions_db.expect_get_variable(id);
if let Some(FreeName::String(free_name)) = &info.free_name {
let result = hook_call(parser, &free_name.to_string());
if result.is_some() {
Expand Down
24 changes: 12 additions & 12 deletions crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,33 +414,33 @@ impl<'parser> JavascriptParser<'parser> {
}

pub fn get_mut_variable_info(&mut self, name: &str) -> Option<&mut VariableInfo> {
let Some(id) = self.definitions_db.get(&self.definitions, name) else {
let Some(id) = self.definitions_db.get(self.definitions, name) else {
return None;
};
Some(self.definitions_db.expect_get_mut_variable(&id))
Some(self.definitions_db.expect_get_mut_variable(id))
}

pub fn get_variable_info(&mut self, name: &str) -> Option<&VariableInfo> {
let Some(id) = self.definitions_db.get(&self.definitions, name) else {
let Some(id) = self.definitions_db.get(self.definitions, name) else {
return None;
};
Some(self.definitions_db.expect_get_variable(&id))
Some(self.definitions_db.expect_get_variable(id))
}

pub fn get_tag_data(&mut self, name: &Atom, tag: &str) -> Option<Box<dyn anymap::CloneAny>> {
self
.get_variable_info(name)
.and_then(|variable_info| variable_info.tag_info)
.and_then(|tag_info_id| {
let mut tag_info = Some(self.definitions_db.expect_get_tag_info(&tag_info_id));
let mut tag_info = Some(self.definitions_db.expect_get_tag_info(tag_info_id));

while let Some(cur_tag_info) = tag_info {
if cur_tag_info.tag == tag {
return cur_tag_info.data.clone();
}
tag_info = cur_tag_info
.next
.map(|tag_info_id| self.definitions_db.expect_get_tag_info(&tag_info_id))
.map(|tag_info_id| self.definitions_db.expect_get_tag_info(tag_info_id))
}

None
Expand All @@ -463,7 +463,7 @@ impl<'parser> JavascriptParser<'parser> {
pub fn get_all_variables_from_current_scope(
&self,
) -> impl Iterator<Item = (&str, &VariableInfoId)> {
let scope = self.definitions_db.expect_get_scope(&self.definitions);
let scope = self.definitions_db.expect_get_scope(self.definitions);
scope.variables()
}

Expand Down Expand Up @@ -505,8 +505,8 @@ impl<'parser> JavascriptParser<'parser> {
data: Option<Data>,
) {
let data = data.map(|data| TagInfoData::into_any(data));
let new_info = if let Some(old_info_id) = self.definitions_db.get(&self.definitions, &name) {
let old_info = self.definitions_db.expect_get_variable(&old_info_id);
let new_info = if let Some(old_info_id) = self.definitions_db.get(self.definitions, &name) {
let old_info = self.definitions_db.expect_get_variable(old_info_id);
if let Some(old_tag_info) = old_info.tag_info {
let declared_scope = old_info.declared_scope;
// FIXME: remove `.clone`
Expand Down Expand Up @@ -879,7 +879,7 @@ impl<'parser> JavascriptParser<'parser> {
}

fn set_strict(&mut self, value: bool) {
let current_scope = self.definitions_db.expect_get_mut_scope(&self.definitions);
let current_scope = self.definitions_db.expect_get_mut_scope(self.definitions);
current_scope.is_strict = value;
}

Expand All @@ -898,13 +898,13 @@ impl<'parser> JavascriptParser<'parser> {
}

pub fn is_strict(&mut self) -> bool {
let scope = self.definitions_db.expect_get_scope(&self.definitions);
let scope = self.definitions_db.expect_get_scope(self.definitions);
scope.is_strict
}

// TODO: remove
pub fn is_unresolved_ident(&mut self, str: &str) -> bool {
self.definitions_db.get(&self.definitions, str).is_none()
self.definitions_db.get(self.definitions, str).is_none()
}
}

Expand Down
6 changes: 3 additions & 3 deletions crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl<'parser> JavascriptParser<'parser> {
let old_in_tagged_template_tag = self.in_tagged_template_tag;

self.in_tagged_template_tag = false;
self.definitions = self.definitions_db.create_child(&old_definitions);
self.definitions = self.definitions_db.create_child(old_definitions);
f(self);

self.definitions = old_definitions;
Expand All @@ -60,7 +60,7 @@ impl<'parser> JavascriptParser<'parser> {

self.in_try = false;
self.in_tagged_template_tag = false;
self.definitions = self.definitions_db.create_child(&old_definitions);
self.definitions = self.definitions_db.create_child(old_definitions);

if has_this {
self.undefined_variable("this".to_string());
Expand All @@ -87,7 +87,7 @@ impl<'parser> JavascriptParser<'parser> {
let old_top_level_scope = self.top_level_scope;
let old_in_tagged_template_tag = self.in_tagged_template_tag;

self.definitions = self.definitions_db.create_child(&old_definitions);
self.definitions = self.definitions_db.create_child(old_definitions);
self.in_tagged_template_tag = false;
if has_this {
self.undefined_variable("this".to_string());
Expand Down
36 changes: 18 additions & 18 deletions crates/rspack_plugin_javascript/src/visitors/scope_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl ScopeInfoDB {
}
}

fn _create(&mut self, parent: Option<&ScopeInfoId>) -> ScopeInfoId {
fn _create(&mut self, parent: Option<ScopeInfoId>) -> ScopeInfoId {
let id = self.next();
let stack = match parent {
Some(parent) => {
Expand Down Expand Up @@ -131,57 +131,57 @@ impl ScopeInfoDB {
self._create(None)
}

pub fn create_child(&mut self, parent: &ScopeInfoId) -> ScopeInfoId {
pub fn create_child(&mut self, parent: ScopeInfoId) -> ScopeInfoId {
self._create(Some(parent))
}

pub fn expect_get_scope(&self, id: &ScopeInfoId) -> &ScopeInfo {
pub fn expect_get_scope(&self, id: ScopeInfoId) -> &ScopeInfo {
self
.map
.get(id)
.get(&id)
.unwrap_or_else(|| panic!("{id:#?} should exist"))
}

pub fn expect_get_mut_scope(&mut self, id: &ScopeInfoId) -> &mut ScopeInfo {
pub fn expect_get_mut_scope(&mut self, id: ScopeInfoId) -> &mut ScopeInfo {
self
.map
.get_mut(id)
.get_mut(&id)
.unwrap_or_else(|| panic!("{id:#?} should exist"))
}

pub fn expect_get_variable(&self, id: &VariableInfoId) -> &VariableInfo {
pub fn expect_get_variable(&self, id: VariableInfoId) -> &VariableInfo {
self
.variable_info_db
.map
.get(id)
.get(&id)
.unwrap_or_else(|| panic!("{id:#?} should exist"))
}

pub fn expect_get_mut_variable(&mut self, id: &VariableInfoId) -> &mut VariableInfo {
pub fn expect_get_mut_variable(&mut self, id: VariableInfoId) -> &mut VariableInfo {
self
.variable_info_db
.map
.get_mut(id)
.get_mut(&id)
.unwrap_or_else(|| panic!("{id:#?} should exist"))
}

pub fn expect_get_tag_info(&self, id: &TagInfoId) -> &TagInfo {
pub fn expect_get_tag_info(&self, id: TagInfoId) -> &TagInfo {
self
.tag_info_db
.map
.get(id)
.get(&id)
.unwrap_or_else(|| panic!("{id:#?} should exist"))
}

pub fn expect_get_mut_tag_info(&mut self, id: &TagInfoId) -> &mut TagInfo {
pub fn expect_get_mut_tag_info(&mut self, id: TagInfoId) -> &mut TagInfo {
self
.tag_info_db
.map
.get_mut(id)
.get_mut(&id)
.unwrap_or_else(|| panic!("{id:#?} should exist"))
}

pub fn get<S: AsRef<str>>(&mut self, id: &ScopeInfoId, key: S) -> Option<VariableInfoId> {
pub fn get<S: AsRef<str>>(&mut self, id: ScopeInfoId, key: S) -> Option<VariableInfoId> {
let definitions = self.expect_get_scope(id);
if let Some(&top_value) = definitions.map.get(key.as_ref()) {
if top_value == VariableInfo::TOMBSTONE || top_value == VariableInfo::UNDEFINED {
Expand All @@ -193,7 +193,7 @@ impl ScopeInfoDB {
for index in (0..definitions.stack.len() - 1).rev() {
// SAFETY: boundary had been checked
let id = unsafe { definitions.stack.get_unchecked(index) };
if let Some(&value) = self.expect_get_scope(id).map.get(key.as_ref()) {
if let Some(&value) = self.expect_get_scope(*id).map.get(key.as_ref()) {
if value == VariableInfo::TOMBSTONE || value == VariableInfo::UNDEFINED {
return None;
} else {
Expand All @@ -212,12 +212,12 @@ impl ScopeInfoDB {
}

pub fn set(&mut self, id: ScopeInfoId, key: String, variable_info_id: VariableInfoId) {
let scope = self.expect_get_mut_scope(&id);
let scope = self.expect_get_mut_scope(id);
scope.map.insert(key, variable_info_id);
}

pub fn delete<S: AsRef<str>>(&mut self, id: ScopeInfoId, key: S) {
let scope = self.expect_get_mut_scope(&id);
let scope = self.expect_get_mut_scope(id);
if scope.stack.len() > 1 {
scope
.map
Expand Down

2 comments on commit 88c6d6a

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
nx ✅ success
rspress ✅ success
rsbuild ✅ success
examples ❌ failure

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-07-19 b3bac1b) Current Change
10000_development-mode + exec 2.26 s ± 16 ms 2.29 s ± 35 ms +1.54 %
10000_development-mode_hmr + exec 699 ms ± 8.9 ms 701 ms ± 7.9 ms +0.17 %
10000_production-mode + exec 2.82 s ± 30 ms 2.89 s ± 27 ms +2.46 %
arco-pro_development-mode + exec 1.92 s ± 80 ms 1.9 s ± 81 ms -0.90 %
arco-pro_development-mode_hmr + exec 434 ms ± 2.6 ms 435 ms ± 4.3 ms +0.42 %
arco-pro_production-mode + exec 3.47 s ± 76 ms 3.5 s ± 119 ms +0.78 %
threejs_development-mode_10x + exec 1.76 s ± 14 ms 1.78 s ± 22 ms +1.25 %
threejs_development-mode_10x_hmr + exec 865 ms ± 4.7 ms 883 ms ± 7 ms +2.10 %
threejs_production-mode_10x + exec 5.73 s ± 30 ms 5.78 s ± 33 ms +0.93 %

Please sign in to comment.