Skip to content

Commit

Permalink
perf: remove unneeded string clone
Browse files Browse the repository at this point in the history
  • Loading branch information
JSerFeng committed Jul 10, 2024
1 parent 81b26f8 commit c234eef
Show file tree
Hide file tree
Showing 21 changed files with 141 additions and 150 deletions.
2 changes: 1 addition & 1 deletion crates/rspack_binding_values/src/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ impl JsCompilation {
request,
public_path,
base_uri,
original_module_context.map(rspack_core::Context::new),
original_module_context.map(rspack_core::Context::from),
)
.await;
match result {
Expand Down
7 changes: 4 additions & 3 deletions crates/rspack_core/src/code_generation_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use anymap::CloneAny;
use rspack_hash::{HashDigest, HashFunction, HashSalt, RspackHash, RspackHashDigest};
use rspack_identifier::IdentifierMap;
use rspack_sources::BoxSource;
use rspack_util::atom::Atom;
use rustc_hash::{FxHashMap as HashMap, FxHashSet};
use serde::Serialize;

Expand Down Expand Up @@ -70,15 +71,15 @@ impl CodeGenerationDataAssetInfo {

#[derive(Clone, Debug)]
pub struct CodeGenerationDataTopLevelDeclarations {
inner: FxHashSet<String>,
inner: FxHashSet<Atom>,
}

impl CodeGenerationDataTopLevelDeclarations {
pub fn new(inner: FxHashSet<String>) -> Self {
pub fn new(inner: FxHashSet<Atom>) -> Self {
Self { inner }
}

pub fn inner(&self) -> &FxHashSet<String> {
pub fn inner(&self) -> &FxHashSet<Atom> {
&self.inner
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;

use rustc_hash::FxHashMap as HashMap;

use super::{factorize::FactorizeTask, MakeTaskContext};
Expand Down Expand Up @@ -36,19 +38,19 @@ impl Task<MakeTaskContext> for ProcessDependenciesTask {
// TODO need implement more dependency `resource_identifier()`
// https://github.com/webpack/webpack/blob/main/lib/Compilation.js#L1621
let id = if let Some(resource_identifier) = module_dependency.resource_identifier() {
resource_identifier.to_string()
Cow::Borrowed(resource_identifier)
} else {
format!(
Cow::Owned(format!(
"{}|{}",
module_dependency.dependency_type(),
module_dependency.request()
)
))
};
Some(id)
} else {
dependency
.as_context_dependency()
.map(|d| ContextDependency::resource_identifier(d).to_string())
.map(|d| Cow::Borrowed(ContextDependency::resource_identifier(d)))
};

if let Some(resource_identifier) = resource_identifier {
Expand Down
10 changes: 2 additions & 8 deletions crates/rspack_core/src/compiler/module_executor/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,7 @@ impl Task<MakeTaskContext> for ExecuteTask {
}
}

tracing::info!(
"modules: {:?}",
&modules.iter().map(|m| m.to_string()).collect::<Vec<_>>()
);
tracing::info!("modules: {:?}", &modules.iter().collect::<Vec<_>>());

let mut chunk_graph = ChunkGraph::default();

Expand Down Expand Up @@ -191,10 +188,7 @@ impl Task<MakeTaskContext> for ExecuteTask {

tracing::info!(
"runtime modules: {:?}",
&runtime_modules
.iter()
.map(|m| m.to_string())
.collect::<Vec<_>>()
&runtime_modules.iter().collect::<Vec<_>>()
);

let mut runtime_module_size = HashMap::default();
Expand Down
104 changes: 51 additions & 53 deletions crates/rspack_core/src/concatenated_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ use crate::{
define_es_module_flag_statement, filter_runtime, impl_source_map_config, merge_runtime_condition,
merge_runtime_condition_non_false, property_access, property_name,
reserved_names::RESERVED_NAMES, returning_function, runtime_condition_expression,
subtract_runtime_condition, AsyncDependenciesBlockIdentifier, BoxDependency, BuildContext,
BuildInfo, BuildMeta, BuildMetaDefaultObject, BuildMetaExportsType, BuildResult,
subtract_runtime_condition, to_identifier, AsyncDependenciesBlockIdentifier, BoxDependency,
BuildContext, BuildInfo, BuildMeta, BuildMetaDefaultObject, BuildMetaExportsType, BuildResult,
ChunkInitFragments, CodeGenerationDataTopLevelDeclarations, CodeGenerationResult, Compilation,
ConcatenatedModuleIdent, ConcatenationScope, ConnectionId, ConnectionState, Context,
DependenciesBlock, DependencyId, DependencyTemplate, DependencyType, ErrorSpan, ExportInfoId,
Expand Down Expand Up @@ -160,10 +160,10 @@ pub struct ConcatenatedModuleInfo {
pub ast: Option<Ast>,
pub source: Option<ReplaceSource<Arc<dyn Source>>>,
pub internal_source: Option<Arc<dyn Source>>,
pub internal_names: HashMap<Atom, String>,
pub internal_names: HashMap<Atom, Atom>,
pub export_map: Option<HashMap<Atom, String>>,
pub raw_export_map: Option<HashMap<Atom, String>>,
pub namespace_object_name: Option<String>,
pub namespace_object_name: Option<Atom>,
pub interop_namespace_object_used: bool,
pub interop_namespace_object_name: Option<Atom>,
pub interop_namespace_object2_used: bool,
Expand Down Expand Up @@ -676,8 +676,8 @@ impl Module for ConcatenatedModule {
module_to_info_map.insert(id, module_info);
}

let mut all_used_names = HashSet::from_iter(RESERVED_NAMES.iter().map(|item| item.to_string()));
let mut top_level_declarations: HashSet<String> = HashSet::default();
let mut all_used_names = HashSet::from_iter(RESERVED_NAMES.iter().map(|s| Atom::new(*s)));
let mut top_level_declarations: HashSet<Atom> = HashSet::default();

for module in modules_with_info.iter() {
let ModuleInfoOrReference::Concatenated(m) = module else {
Expand All @@ -695,17 +695,17 @@ impl Module for ConcatenatedModule {
for ident in collector.ids {
if ident.id.span.ctxt == info.global_ctxt {
info.global_scope_ident.push(ident.clone());
all_used_names.insert(ident.id.sym.to_string());
all_used_names.insert(ident.id.sym.clone());
}
if ident.is_class_expr_with_ident {
all_used_names.insert(ident.id.sym.to_string());
all_used_names.insert(ident.id.sym.clone());
continue;
}
// deconflict naming from inner scope, the module level deconflict will be finished
// you could see tests/webpack-test/cases/scope-hoisting/renaming-4967 as a example
// during module eval phase.
if ident.id.span.ctxt != info.module_ctxt {
all_used_names.insert(ident.id.sym.to_string());
all_used_names.insert(ident.id.sym.clone());
}
info.idents.push(ident);
}
Expand Down Expand Up @@ -733,7 +733,7 @@ impl Module for ConcatenatedModule {
let module = module_graph
.module_by_identifier(&info.id())
.expect("should have module identifier");
let readable_identifier = module.readable_identifier(&context).to_string();
let readable_identifier = module.readable_identifier(&context);
let exports_type: Option<BuildMetaExportsType> =
module.build_meta().map(|item| item.exports_type);
let default_object: Option<BuildMetaDefaultObject> =
Expand All @@ -749,13 +749,13 @@ impl Module for ConcatenatedModule {
continue;
}
// Check if the name is already used
if all_used_names.contains(name.as_str()) {
if all_used_names.contains(name) {
// Find a new name and update references
let new_name = find_new_name(name, &all_used_names, None, &readable_identifier);
// dbg!(&name, &new_name);
all_used_names.insert(new_name.clone());
info.internal_names.insert(name.clone(), new_name.clone());
top_level_declarations.insert(new_name.as_str().into());
top_level_declarations.insert(new_name.clone());

// Update source
let source = info.source.as_mut().expect("should have source");
Expand All @@ -773,9 +773,9 @@ impl Module for ConcatenatedModule {
}
} else {
// Handle the case when the name is not already used
all_used_names.insert(name.to_string());
info.internal_names.insert(name.clone(), name.to_string());
top_level_declarations.insert(name.to_string());
all_used_names.insert(name.clone());
info.internal_names.insert(name.clone(), name.clone());
top_level_declarations.insert(name.clone());
}
}

Expand All @@ -791,7 +791,6 @@ impl Module for ConcatenatedModule {
&readable_identifier,
))
};

if let Some(namespace_object_name) = namespace_object_name {
all_used_names.insert(namespace_object_name.clone());
info.namespace_object_name = Some(namespace_object_name.clone());
Expand All @@ -802,48 +801,48 @@ impl Module for ConcatenatedModule {

// Handle external type
ModuleInfo::External(info) => {
let external_name = find_new_name("", &all_used_names, None, &readable_identifier);
let external_name: Atom = find_new_name("", &all_used_names, None, &readable_identifier);
all_used_names.insert(external_name.clone());
info.name = Some(external_name.as_str().into());
top_level_declarations.insert(external_name.as_str().into());
info.name = Some(external_name.clone());
top_level_declarations.insert(external_name.clone());
}
}
// Handle additional logic based on module build meta
if exports_type != Some(BuildMetaExportsType::Namespace) {
let external_name_interop = find_new_name(
let external_name_interop: Atom = find_new_name(
"namespaceObject",
&all_used_names,
None,
&readable_identifier,
);
all_used_names.insert(external_name_interop.as_str().into());
info.set_interop_namespace_object_name(Some(external_name_interop.as_str().into()));
top_level_declarations.insert(external_name_interop.as_str().into());
all_used_names.insert(external_name_interop.clone());
info.set_interop_namespace_object_name(Some(external_name_interop.clone()));
top_level_declarations.insert(external_name_interop.clone());
}

if exports_type == Some(BuildMetaExportsType::Default)
&& !matches!(default_object, Some(BuildMetaDefaultObject::Redirect))
{
let external_name_interop = find_new_name(
let external_name_interop: Atom = find_new_name(
"namespaceObject2",
&all_used_names,
None,
&readable_identifier,
);
all_used_names.insert(external_name_interop.as_str().into());
info.set_interop_namespace_object2_name(Some(external_name_interop.as_str().into()));
top_level_declarations.insert(external_name_interop.as_str().into());
all_used_names.insert(external_name_interop.clone());
info.set_interop_namespace_object2_name(Some(external_name_interop.clone()));
top_level_declarations.insert(external_name_interop.clone());
}

if matches!(
exports_type,
Some(BuildMetaExportsType::Dynamic | BuildMetaExportsType::Unset)
) {
let external_name_interop =
let external_name_interop: Atom =
find_new_name("default", &all_used_names, None, &readable_identifier);
all_used_names.insert(external_name_interop.clone());
info.set_interop_default_access_name(Some(external_name_interop.as_str().into()));
top_level_declarations.insert(external_name_interop.as_str().into());
info.set_interop_default_access_name(Some(external_name_interop.clone()));
top_level_declarations.insert(external_name_interop.clone());
}
}

Expand Down Expand Up @@ -1065,7 +1064,7 @@ impl Module for ConcatenatedModule {
let box_module = module_graph
.module_by_identifier(module_info_id)
.expect("should have box module");
let module_readable_identifier = box_module.readable_identifier(&context).to_string();
let module_readable_identifier = box_module.readable_identifier(&context);
let strict_harmony_module = box_module
.build_meta()
.map(|meta| meta.strict_harmony_module)
Expand Down Expand Up @@ -1187,7 +1186,7 @@ impl Module for ConcatenatedModule {
let box_module = module_graph
.module_by_identifier(&info.id())
.expect("should have box module");
let module_readable_identifier = box_module.readable_identifier(&context).to_string();
let module_readable_identifier = box_module.readable_identifier(&context);

match info {
ModuleInfo::Concatenated(info) => {
Expand Down Expand Up @@ -1238,7 +1237,7 @@ impl Module for ConcatenatedModule {
.expect("should have module id")
)));

name = info.name.as_ref().map(|atom| atom.to_string());
name = info.name.clone();
}
}

Expand Down Expand Up @@ -1675,15 +1674,15 @@ impl ConcatenatedModule {
let source = inner
.remove(&SourceType::JavaScript)
.expect("should have javascript source");
let source_code = source.source().to_string();
let source_code = source.source();

let cm: Arc<swc_core::common::SourceMap> = Default::default();
let fm = cm.new_source_file(
FileName::Custom(format!(
"{}",
self.readable_identifier(&compilation.options.context),
)),
source_code,
source_code.into(),
);
let comments = SwcComments::default();
let mut module_info = concatenation_scope.current_module;
Expand Down Expand Up @@ -1983,8 +1982,7 @@ impl ConcatenatedModule {
raw_name: info
.namespace_object_name
.clone()
.expect("should have namespace_object_name")
.into(),
.expect("should have namespace_object_name"),
ids: export_name.clone(),
export_name,
info_id: info.module,
Expand Down Expand Up @@ -2033,8 +2031,7 @@ impl ConcatenatedModule {
raw_name: info
.namespace_object_name
.clone()
.expect("should have namespace_object_name")
.into(),
.expect("should have namespace_object_name"),
ids: export_name.clone(),
export_name,
info_id: info.module,
Expand Down Expand Up @@ -2063,7 +2060,7 @@ impl ConcatenatedModule {
});
} else {
return Binding::Raw(RawBinding {
raw_name: "/* unused export */ undefined".to_string().into(),
raw_name: "/* unused export */ undefined".into(),
ids: export_name[1..].to_vec(),
export_name,
info_id: info.module,
Expand Down Expand Up @@ -2162,7 +2159,7 @@ impl ConcatenatedModule {
let comment = if used_name == export_name {
"".to_string()
} else {
Template::to_normal_comment(&join_atom(export_name.iter(), ",").to_string())
Template::to_normal_comment(&join_atom(export_name.iter(), ","))
};
Binding::Raw(RawBinding {
raw_name: format!(
Expand All @@ -2178,7 +2175,7 @@ impl ConcatenatedModule {
})
} else {
Binding::Raw(RawBinding {
raw_name: "/* unused export */ undefined".to_string().into(),
raw_name: "/* unused export */ undefined".into(),
ids: export_name[1..].to_vec(),
export_name,
info_id: info.module,
Expand Down Expand Up @@ -2238,25 +2235,26 @@ pub fn map_box_diagnostics_to_module_parse_diagnostics(

pub fn find_new_name(
old_name: &str,
used_names1: &HashSet<String>,
used_names2: Option<&HashSet<String>>,
used_names1: &HashSet<Atom>,
used_names2: Option<&HashSet<Atom>>,
extra_info: &str,
) -> String {
let mut name = old_name.to_string();
) -> Atom {
let mut name = Cow::Borrowed(old_name);

if name == DEFAULT_EXPORT {
name = String::new();
name = Cow::Borrowed("");
}
if name == NAMESPACE_OBJECT_EXPORT {
name = "namespaceObject".to_string();
name = Cow::Borrowed("namespaceObject");
}

// Remove uncool stuff
let extra_info = REGEX.replace_all(extra_info, "").to_string();
let extra_info = REGEX.replace_all(extra_info, "");

let mut splitted_info: Vec<&str> = extra_info.split('/').collect();
while let Some(info_part) = splitted_info.pop() {
name = format!("{}_{}", info_part, name);
let name_ident = Template::to_identifier(&name);
name = Cow::Owned(format!("{}_{}", info_part, name));
let name_ident = to_identifier(&name).into();
if !used_names1.contains(&name_ident)
&& (used_names2.is_none()
|| !used_names2
Expand All @@ -2268,14 +2266,14 @@ pub fn find_new_name(
}

let mut i = 0;
let mut name_with_number = Template::to_identifier(&format!("{}_{}", name, i));
let mut name_with_number = to_identifier(&format!("{}_{}", name, i)).into();
while used_names1.contains(&name_with_number)
|| used_names2
.map(|map| map.contains(&name_with_number))
.unwrap_or_default()
{
i += 1;
name_with_number = Template::to_identifier(&format!("{}_{}", name, i));
name_with_number = to_identifier(&format!("{}_{}", name, i)).into();
}

name_with_number
Expand Down
Loading

0 comments on commit c234eef

Please sign in to comment.