这是indexloc提供的服务,不要输入任何密码
Skip to content

Optimize {run_add_to,run_remove_from}_scope_queue #2423

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

Merged
merged 5 commits into from
Oct 28, 2022

Conversation

alexkirsz
Copy link
Contributor

This PR does two things:

  • avoids allocating a new vector in add_to_scope_internal_shallow/remove_from_scope_internal_shallow
  • use an actual queue instead of a stack for these operations. This part doesn't change much, if anything.

This shows improvements up to ~6% on a 5000 modules SSR benchmark:
image

On 20k modules, there are no significant changes.

@vercel
Copy link

vercel bot commented Oct 27, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
examples-basic-web 🔄 Building (Inspect) Oct 27, 2022 at 10:20PM (UTC)
5 Ignored Deployments
Name Status Preview Comments Updated
examples-designsystem-docs ⬜️ Ignored (Inspect) Oct 27, 2022 at 10:20PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Oct 27, 2022 at 10:20PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Oct 27, 2022 at 10:20PM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Oct 27, 2022 at 10:20PM (UTC)
turbo-site ⬜️ Ignored (Inspect) Visit Preview Oct 27, 2022 at 10:20PM (UTC)

@alexkirsz alexkirsz requested review from sokra, jridgewell and ForsakenHarmony and removed request for sokra October 27, 2022 19:11
let schedule_self =
self.add_self_to_new_scope(&mut state, id, backend, turbo_tasks);
drop(state);
queue.extend(children.iter().copied().map(|child| (child, depth + 1)));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line is the actual change.

);
});
if queue.len() > SPLIT_OFF_QUEUE_AT {
let split_off_queue = queue.split_off(SPLIT_OFF_QUEUE_AT);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This logic is now much simpler.

@@ -859,17 +860,11 @@ impl Task {
TaskScopes::Inner(ref mut set, _) => {
if set.remove(id) {
self.remove_self_from_scope(&mut state, id, backend, turbo_tasks);
let children = state.children.iter().copied().collect::<Vec<_>>();
queue.extend(state.children.iter().copied());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line is the actual change.

Copy link
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

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

Much better!

@alexkirsz alexkirsz added the pr: automerge Kodiak will merge these automatically after checks pass label Oct 27, 2022
@jridgewell jridgewell merged commit 95f8ffa into main Oct 28, 2022
@jridgewell jridgewell deleted the alexkirsz/optimize-add-remove-scope branch October 28, 2022 04:38
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
)

* Optimize {run_add_to,run_remove_from}_scope_queue

* Use an actual queue instead of a stack

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
)

* Optimize {run_add_to,run_remove_from}_scope_queue

* Use an actual queue instead of a stack

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants