Skip to content

Commit

Permalink
refactor: remove connection id (#8089)
Browse files Browse the repository at this point in the history
  • Loading branch information
jerrykingxyz authored Oct 12, 2024
1 parent aad975b commit cc3c68c
Show file tree
Hide file tree
Showing 27 changed files with 292 additions and 545 deletions.
52 changes: 21 additions & 31 deletions crates/rspack_core/src/build_chunk_graph/code_splitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet, FxHasher};

use crate::dependencies_block::AsyncDependenciesToInitialChunkError;
use crate::{
add_connection_states, assign_depth, assign_depths, get_entry_runtime, merge_runtime,
AsyncDependenciesBlockIdentifier, ChunkGroup, ChunkGroupKind, ChunkGroupOptions, ChunkGroupUkey,
ChunkLoading, ChunkUkey, Compilation, ConnectionId, ConnectionState, DependenciesBlock,
DependencyLocation, EntryDependency, EntryRuntime, GroupOptions, Logger, ModuleDependency,
ModuleGraph, ModuleIdentifier, RuntimeSpec, SyntheticDependencyLocation,
assign_depth, assign_depths, get_entry_runtime, merge_runtime, AsyncDependenciesBlockIdentifier,
ChunkGroup, ChunkGroupKind, ChunkGroupOptions, ChunkGroupUkey, ChunkLoading, ChunkUkey,
Compilation, ConnectionState, DependenciesBlock, DependencyId, DependencyLocation,
EntryDependency, EntryRuntime, GroupOptions, Logger, ModuleDependency, ModuleGraph,
ModuleIdentifier, RuntimeSpec, SyntheticDependencyLocation,
};

type IndexMap<K, V, H = FxHasher> = RawIndexMap<K, V, BuildHasherDefault<H>>;
Expand All @@ -39,7 +39,7 @@ pub struct ChunkGroupInfo {
pub available_modules_to_be_merged: Vec<BigUint>,

pub skipped_items: IdentifierIndexSet,
pub skipped_module_connections: IndexSet<(ModuleIdentifier, Vec<ConnectionId>)>,
pub skipped_module_connections: IndexSet<(ModuleIdentifier, Vec<DependencyId>)>,
// set of children chunk groups, that will be revisited when available_modules shrink
pub children: UkeyIndexSet<CgiUkey>,
// set of chunk groups that are the source for min_available_modules
Expand Down Expand Up @@ -152,7 +152,7 @@ type BlockModulesRuntimeMap = IndexMap<
OptionalRuntimeSpec,
IndexMap<
DependenciesBlockIdentifier,
Vec<(ModuleIdentifier, ConnectionState, Vec<ConnectionId>)>,
Vec<(ModuleIdentifier, ConnectionState, Vec<DependencyId>)>,
>,
>;

Expand Down Expand Up @@ -278,24 +278,24 @@ fn add_chunk_in_group(
}

fn get_active_state_of_connections(
connections: &[ConnectionId],
connections: &[DependencyId],
runtime: Option<&RuntimeSpec>,
module_graph: &ModuleGraph,
) -> ConnectionState {
let mut iter = connections.iter();
let id = iter.next().expect("should have connection");
let mut merged = module_graph
.connection_by_connection_id(id)
.connection_by_dependency_id(id)
.expect("should have connection")
.get_active_state(module_graph, runtime);
.active_state(module_graph, runtime);
if merged.is_true() {
return merged;
}
for c in iter {
let c = module_graph
.connection_by_connection_id(c)
.connection_by_dependency_id(c)
.expect("should have connection");
merged = add_connection_states(merged, c.get_active_state(module_graph, runtime));
merged = merged + c.active_state(module_graph, runtime);
if merged.is_true() {
return merged;
}
Expand Down Expand Up @@ -1467,7 +1467,7 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
&mut self,
module: DependenciesBlockIdentifier,
runtime: Option<&RuntimeSpec>,
) -> Vec<(ModuleIdentifier, ConnectionState, Vec<ConnectionId>)> {
) -> Vec<(ModuleIdentifier, ConnectionState, Vec<DependencyId>)> {
if let Some(modules) = self
.block_modules_runtime_map
.get::<OptionalRuntimeSpec>(&runtime.cloned().into())
Expand Down Expand Up @@ -1500,34 +1500,24 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on

let sorted_connections = module_graph
.get_ordered_connections(&module)
.expect("should have module")
.into_iter()
.map(|conn_id| {
let conn = module_graph
.connection_by_connection_id(conn_id)
.expect("should have connection");

let dep = module_graph
.dependency_by_id(&conn.dependency_id)
.expect("should have dependency");

(dep, conn_id)
});
.expect("should have module");

// keep the dependency order sorted by span
let mut connection_map: IndexMap<
(DependenciesBlockIdentifier, ModuleIdentifier),
Vec<ConnectionId>,
Vec<DependencyId>,
> = IndexMap::default();

for (dep, connection_id) in sorted_connections {
for dep_id in sorted_connections {
let dep = module_graph
.dependency_by_id(dep_id)
.expect("should have dep");
if dep.as_module_dependency().is_none() && dep.as_context_dependency().is_none() {
continue;
}
if matches!(dep.as_module_dependency().map(|d| d.weak()), Some(true)) {
continue;
}
let dep_id = dep.id();
// Dependency created but no module is available.
// This could happen when module factorization is failed, but `options.bail` set to `false`
let module_graph = self.compilation.get_module_graph();
Expand All @@ -1542,8 +1532,8 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
};
connection_map
.entry((block_id, *module_identifier))
.and_modify(|e| e.push(*connection_id))
.or_insert_with(|| vec![*connection_id]);
.and_modify(|e| e.push(*dep_id))
.or_insert_with(|| vec![*dep_id]);
}

for ((block_id, module_identifier), connections) in connection_map {
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/chunk_graph/chunk_graph_chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ impl ChunkGraph {
) {
for connection in module_graph.get_outgoing_connections(&module) {
// https://github.com/webpack/webpack/blob/1f99ad6367f2b8a6ef17cce0e058f7a67fb7db18/lib/ChunkGraph.js#L290
let active_state = connection.get_active_state(module_graph, None);
let active_state = connection.active_state(module_graph, None);
match active_state {
crate::ConnectionState::Bool(false) => {
continue;
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/chunk_graph/chunk_graph_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl ChunkGraph {
if visited_modules.contains(module_identifier) {
continue;
}
if connection.get_active_state(&mg, runtime).is_false() {
if connection.active_state(&mg, runtime).is_false() {
continue;
}
visited_modules.insert(*module_identifier);
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/compiler/make/cutout/fix_issuers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl FixIssuers {
.expect("should have module graph module");
self
.origin_module_issuers
.insert(*module_identifier, mgm.get_issuer().clone());
.insert(*module_identifier, mgm.issuer().clone());
}

pub fn fix_artifact(self, artifact: &mut MakeArtifact) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl ModuleDeps {
.dependency_by_id(dep_id)
.expect("should have dependency");

let Some(conn) = module_graph.connection_by_dependency(dep_id) else {
let Some(conn) = module_graph.connection_by_dependency_id(dep_id) else {
continue;
};
let identifier = conn.module_identifier();
Expand Down
5 changes: 2 additions & 3 deletions crates/rspack_core/src/compiler/make/cutout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,11 @@ impl Cutout {
let mut module_graph = artifact.get_module_graph_mut();
for dep_id in remove_entry_deps {
// connection may have been deleted by revoke module
if let Some(con) = module_graph.connection_by_dependency(&dep_id) {
if let Some(con) = module_graph.connection_by_dependency_id(&dep_id) {
self
.clean_isolated_module
.add_need_check_module(*con.module_identifier());
let con_id = con.id;
module_graph.revoke_connection(&con_id, true);
module_graph.revoke_connection(&dep_id, true);
}
force_build_deps.remove(&(dep_id, None));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/compiler/make/repair/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl Task<MakeTaskContext> for AddTask {
if self.module.as_self_module().is_some() {
let issuer = self
.module_graph_module
.get_issuer()
.issuer()
.identifier()
.expect("self module should have issuer");

Expand Down
4 changes: 2 additions & 2 deletions crates/rspack_core/src/compiler/module_executor/ctrl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,9 @@ impl Task<MakeTaskContext> for FinishModuleTask {
.expect("should have mgm");

let mut original_module_identifiers = HashSet::default();
for connection_id in mgm.incoming_connections() {
for dep_id in mgm.incoming_connections() {
let connection = module_graph
.connection_by_connection_id(connection_id)
.connection_by_dependency_id(dep_id)
.expect("should have connection");
if let Some(original_module_identifier) = &connection.original_module_identifier {
// skip self reference
Expand Down
22 changes: 9 additions & 13 deletions crates/rspack_core/src/concatenated_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,12 @@ use crate::{
subtract_runtime_condition, to_identifier, AsyncDependenciesBlockIdentifier, BoxDependency,
BuildContext, BuildInfo, BuildMeta, BuildMetaDefaultObject, BuildMetaExportsType, BuildResult,
ChunkInitFragments, CodeGenerationDataTopLevelDeclarations, CodeGenerationExportsFinalNames,
CodeGenerationResult, Compilation, ConcatenatedModuleIdent, ConcatenationScope, ConnectionId,
ConnectionState, Context, DependenciesBlock, DependencyId, DependencyTemplate, DependencyType,
ErrorSpan, ExportInfo, ExportInfoProvided, ExportsArgument, ExportsType, FactoryMeta,
IdentCollector, LibIdentOptions, Module, ModuleDependency, ModuleGraph, ModuleGraphConnection,
ModuleIdentifier, ModuleLayer, ModuleType, Resolve, RuntimeCondition, RuntimeGlobals,
RuntimeSpec, SourceType, SpanExt, Template, UsageState, UsedName, DEFAULT_EXPORT,
NAMESPACE_OBJECT_EXPORT,
CodeGenerationResult, Compilation, ConcatenatedModuleIdent, ConcatenationScope, ConnectionState,
Context, DependenciesBlock, DependencyId, DependencyTemplate, DependencyType, ErrorSpan,
ExportInfo, ExportInfoProvided, ExportsArgument, ExportsType, FactoryMeta, IdentCollector,
LibIdentOptions, Module, ModuleDependency, ModuleGraph, ModuleGraphConnection, ModuleIdentifier,
ModuleLayer, ModuleType, Resolve, RuntimeCondition, RuntimeGlobals, RuntimeSpec, SourceType,
SpanExt, Template, UsageState, UsedName, DEFAULT_EXPORT, NAMESPACE_OBJECT_EXPORT,
};

type ExportsDefinitionArgs = Vec<(String, String)>;
Expand Down Expand Up @@ -140,14 +139,14 @@ struct ConcatenationEntryConcatenated {

#[derive(Debug)]
struct ConcatenationEntryExternal {
connection: ConnectionId,
dependency: DependencyId,
runtime_condition: RuntimeCondition,
}

impl ConcatenationEntryExternal {
pub fn module(&self, mg: &ModuleGraph) -> ModuleIdentifier {
let con = mg
.connection_by_connection_id(&self.connection)
.connection_by_dependency_id(&self.dependency)
.expect("should have connection");
*con.module_identifier()
}
Expand Down Expand Up @@ -1580,11 +1579,8 @@ impl ConcatenatedModule {
merge_runtime_condition(&last.runtime_condition, &runtime_condition, runtime);
return;
}
let con_id = mg
.connection_id_by_dependency_id(&con.dependency_id)
.expect("should have dep id");
list.push(ConcatenationEntry::External(ConcatenationEntryExternal {
connection: *con_id,
dependency: con.dependency_id,
runtime_condition,
}));
}
Expand Down
Loading

2 comments on commit cc3c68c

@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-10-12 3153da6) Current Change
10000_development-mode + exec 2.17 s ± 32 ms 2.13 s ± 34 ms -1.66 %
10000_development-mode_hmr + exec 684 ms ± 15 ms 663 ms ± 21 ms -2.97 %
10000_production-mode + exec 2.73 s ± 33 ms 2.67 s ± 38 ms -2.08 %
arco-pro_development-mode + exec 1.86 s ± 57 ms 1.81 s ± 65 ms -2.53 %
arco-pro_development-mode_hmr + exec 436 ms ± 6.7 ms 428 ms ± 5.3 ms -1.74 %
arco-pro_production-mode + exec 3.13 s ± 78 ms 3.08 s ± 75 ms -1.36 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.15 s ± 77 ms 3.14 s ± 75 ms -0.50 %
threejs_development-mode_10x + exec 1.69 s ± 19 ms 1.69 s ± 18 ms -0.20 %
threejs_development-mode_10x_hmr + exec 804 ms ± 18 ms 795 ms ± 8.9 ms -1.22 %
threejs_production-mode_10x + exec 5.03 s ± 33 ms 5.04 s ± 35 ms +0.10 %

@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
rspress ✅ success
rslib ✅ success
rsbuild ✅ success
examples ✅ success
devserver ✅ success

Please sign in to comment.