Skip to content
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

perf: reduce allocation for parsing #7219

Merged
merged 1 commit into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion crates/rspack_identifier/src/lib.rs
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())
}
}
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
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
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
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
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
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
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
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
Loading