Skip to content

refactor: remove connection id#8089

Merged
jerrykingxyz merged 1 commit intomainfrom
jerry/module_graph
Oct 12, 2024
Merged

refactor: remove connection id#8089
jerrykingxyz merged 1 commit intomainfrom
jerry/module_graph

Conversation

@jerrykingxyz
Copy link
Copy Markdown
Contributor

@jerrykingxyz jerrykingxyz commented Oct 11, 2024

Summary

Currently we use ModuleIdentifier to access Module and ModuleGraphModule, but need to convert the dependency_id to the connection_id to access the Connection, which will cause additional performance loss.

impl ModuleGraph {
    fn connection_by_dependency_id(&self, dep_id: &DependencyId) -> Option<Connection> {
        let con_id = self.dependency_id_to_connection_id.get(dep_id)?;
        self.connections.get(con_id)
    }
}

This PR will remove ConnectionId and use DependencyId to access Connection.

impl ModuleGraph {
    fn connection_by_dependency_id(&self, dep_id: &DependencyId) -> Option<Connection> {
        self.connections.get(dep_id)
    }
}

NOTE: There are some inactive connections in the module graph that cannot be found from the dependency ID. This is useful when you need to print debug information, but it makes the module graph more cluttered. We will use some other methods to ensure these features.

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@jerrykingxyz
Copy link
Copy Markdown
Contributor Author

!bench

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Oct 11, 2024
@netlify
Copy link
Copy Markdown

netlify bot commented Oct 11, 2024

Deploy Preview for rspack ready!

Name Link
🔨 Latest commit bfb5f76
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/670a23fd7ff2a8000873b80f
😎 Deploy Preview https://deploy-preview-8089--rspack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rspack-bot
Copy link
Copy Markdown

⏳ Triggered benchmark: Open

@jerrykingxyz jerrykingxyz force-pushed the jerry/module_graph branch 3 times, most recently from c2ac5ee to 73f5ace Compare October 11, 2024 09:44
@jerrykingxyz
Copy link
Copy Markdown
Contributor Author

!bench

@rspack-bot
Copy link
Copy Markdown

rspack-bot commented Oct 11, 2024

📝 Benchmark detail: Open

Name Base (2024-10-11 598d44e) Current Change
10000_development-mode + exec 2.16 s ± 42 ms 2.14 s ± 22 ms -0.66 %
10000_development-mode_hmr + exec 682 ms ± 13 ms 662 ms ± 16 ms -2.97 %
10000_production-mode + exec 2.74 s ± 39 ms 2.68 s ± 29 ms -1.98 %
arco-pro_development-mode + exec 1.84 s ± 93 ms 1.81 s ± 81 ms -1.13 %
arco-pro_development-mode_hmr + exec 436 ms ± 6.2 ms 427 ms ± 2.1 ms -2.18 %
arco-pro_production-mode + exec 3.14 s ± 85 ms 3.13 s ± 74 ms -0.21 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.13 s ± 85 ms 3.19 s ± 117 ms +1.80 %
threejs_development-mode_10x + exec 1.7 s ± 16 ms 1.68 s ± 16 ms -0.81 %
threejs_development-mode_10x_hmr + exec 802 ms ± 9 ms 792 ms ± 15 ms -1.29 %
threejs_production-mode_10x + exec 5.04 s ± 24 ms 5.02 s ± 21 ms -0.34 %

@jerrykingxyz
Copy link
Copy Markdown
Contributor Author

!bench

@rspack-bot
Copy link
Copy Markdown

rspack-bot commented Oct 12, 2024

📝 Benchmark detail: Open

Name Base (2024-10-12 3153da6) Current Change
10000_development-mode + exec 2.17 s ± 32 ms 2.15 s ± 36 ms -0.73 %
10000_development-mode_hmr + exec 684 ms ± 15 ms 665 ms ± 13 ms -2.71 %
10000_production-mode + exec 2.73 s ± 33 ms 2.68 s ± 29 ms -1.87 %
arco-pro_development-mode + exec 1.86 s ± 57 ms 1.81 s ± 78 ms -2.60 %
arco-pro_development-mode_hmr + exec 436 ms ± 6.7 ms 428 ms ± 3.2 ms -1.74 %
arco-pro_production-mode + exec 3.13 s ± 78 ms 3.12 s ± 83 ms -0.33 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.15 s ± 77 ms 3.15 s ± 78 ms +0.05 %
threejs_development-mode_10x + exec 1.69 s ± 19 ms 1.69 s ± 19 ms +0.07 %
threejs_development-mode_10x_hmr + exec 804 ms ± 18 ms 797 ms ± 5 ms -0.90 %
threejs_production-mode_10x + exec 5.03 s ± 33 ms 5.04 s ± 41 ms +0.13 %

@jerrykingxyz jerrykingxyz merged commit cc3c68c into main Oct 12, 2024
@jerrykingxyz jerrykingxyz deleted the jerry/module_graph branch October 12, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants