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

refactor: remove connection id #8089

Merged
merged 1 commit into from
Oct 12, 2024
Merged

refactor: remove connection id #8089

merged 1 commit into from
Oct 12, 2024

Conversation

jerrykingxyz
Copy link
Collaborator

@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
Collaborator Author

!bench

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

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

⏳ 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
Collaborator Author

!bench

@rspack-bot
Copy link

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
Collaborator Author

!bench

@rspack-bot
Copy link

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
33 checks passed
@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