Skip to content

perf: get resolve in external plugin#11111

Merged
SyMind merged 6 commits into
mainfrom
perf-external-get-resolve
Jul 21, 2025
Merged

perf: get resolve in external plugin#11111
SyMind merged 6 commits into
mainfrom
perf-external-get-resolve

Conversation

@SyMind

@SyMind SyMind commented Jul 21, 2025

Copy link
Copy Markdown
Member

Summary

Directly implement the getResolve method through NAPI to avoid the overhead of constructing JsResolver multiple times.

In this Next.js benchmark case https://github.com/SyMind/shadcn-ui/tree/next-rspack, build performance improved from 17s to 16s.

Before

image

After

image

Related links

Checklist

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

@netlify

netlify Bot commented Jul 21, 2025

Copy link
Copy Markdown

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit dd13d32
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/687e0b15007236000857cb1a

@github-actions github-actions Bot added release: performance release: performance related release(mr only) team The issue/pr is created by the member of Rspack. labels Jul 21, 2025
@github-actions

github-actions Bot commented Jul 21, 2025

Copy link
Copy Markdown
Contributor

📦 Binary Size-limit

Comparing dd13d32 to fix: emit error for esm duplicate export (#11112) by Gengkun

❌ Size increased by 15.38KB from 48.91MB to 48.92MB (⬆️0.03%)

@SyMind
SyMind force-pushed the perf-external-get-resolve branch from 424cc2d to eb61964 Compare July 21, 2025 08:19
@codspeed-hq

codspeed-hq Bot commented Jul 21, 2025

Copy link
Copy Markdown
Contributor

CodSpeed Performance Report

Merging #11111 will not alter performance

Comparing perf-external-get-resolve (dd13d32) with main (46c867e)

Summary

✅ 16 untouched benchmarks

@SyMind
SyMind requested a review from Copilot July 21, 2025 09:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR optimizes performance in the external plugin by replacing the Resolver-based approach with a more efficient resolve mechanism. The change aims to improve resolution performance when handling external dependencies.

  • Replaces Resolver class usage with getRawResolve adapter function
  • Implements direct resolution using ctx.getResolve() instead of creating new Resolver instances
  • Moves resolution logic from TypeScript to Rust for better performance

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/rspack/src/builtin-plugin/ExternalsPlugin.ts Refactors getResolve method to use new adapter-based resolution approach
crates/rspack_binding_api/src/raw_options/raw_external.rs Adds native Rust implementation of get_resolve method with improved performance
Comments suppressed due to low confidence (3)

packages/rspack/src/builtin-plugin/ExternalsPlugin.ts:83

  • [nitpick] Variable names 'res' and 'rej' are abbreviated. Consider using 'resolve' and 'reject' for better readability.
									return new Promise((res, rej) => {

crates/rspack_binding_api/src/raw_options/raw_external.rs:122

  • [nitpick] Variable name 'first' is ambiguous. Consider a more descriptive name like 'base_resolve_options' or 'default_resolve_options'.
    let first = Arc::new(self.resolve_options_with_dependency_type.clone());

crates/rspack_binding_api/src/raw_options/raw_external.rs:123

  • [nitpick] Variable name 'second' is ambiguous. Consider a more descriptive name like 'merged_resolve_options' or 'override_resolve_options'.
    let second = Arc::new(

Comment thread packages/rspack/src/builtin-plugin/ExternalsPlugin.ts Outdated
Comment thread packages/rspack/src/builtin-plugin/ExternalsPlugin.ts Outdated
@SyMind
SyMind marked this pull request as ready for review July 21, 2025 09:54
@SyMind
SyMind requested review from h-a-n-a and hardfist as code owners July 21, 2025 09:54
@SyMind
SyMind merged commit 16ec430 into main Jul 21, 2025
48 checks passed
@SyMind
SyMind deleted the perf-external-get-resolve branch July 21, 2025 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: performance release: performance related release(mr only) 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