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

[turbo-trace] add additional filter modes for allocs over time #7673

Conversation

arlyon
Copy link
Contributor

@arlyon arlyon commented Mar 8, 2024

Description

This adds a few modes to see allocations per unit time. The hierarchy is a bit hard but by selecting the one you want to inspect and changing the view for sub-items you can usually find some useful data.

Testing Instructions

Closes TURBO-2574

Copy link

vercel bot commented Mar 8, 2024

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

Name Status Preview Comments Updated (UTC)
examples-designsystem-docs 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 22, 2024 7:39am
examples-native-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 22, 2024 7:39am
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 7:39am
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 7:39am
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 7:39am
6 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:39am
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:39am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:39am
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:39am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:39am
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 7:39am

Copy link
Contributor Author

arlyon commented Mar 8, 2024

Copy link
Contributor

github-actions bot commented Mar 8, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

github-actions bot commented Mar 8, 2024

✅ This change can build next-swc

Copy link
Contributor

github-actions bot commented Mar 8, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

@arlyon arlyon force-pushed the 03-08-add_additional_filter_modes_for_allocation_and_alloc_count_over_time branch from f7624f1 to cc8a7d1 Compare March 8, 2024 09:01
@arlyon arlyon force-pushed the 03-08-add_additional_filter_modes_for_allocation_and_alloc_count_over_time branch from cc8a7d1 to 6c2de7a Compare March 19, 2024 14:50
@arlyon arlyon marked this pull request as ready for review March 19, 2024 14:51
@arlyon arlyon requested a review from a team as a code owner March 19, 2024 14:51
@arlyon arlyon force-pushed the 03-08-add_additional_filter_modes_for_allocation_and_alloc_count_over_time branch from 6c2de7a to a959ee1 Compare March 19, 2024 16:37
@arlyon arlyon force-pushed the 03-08-add_additional_filter_modes_for_allocation_and_alloc_count_over_time branch from a959ee1 to 56157e9 Compare April 24, 2024 19:10
@arlyon arlyon force-pushed the 03-08-add_additional_filter_modes_for_allocation_and_alloc_count_over_time branch from 56157e9 to 1557197 Compare May 15, 2024 11:24
@arlyon arlyon force-pushed the 03-08-add_additional_filter_modes_for_allocation_and_alloc_count_over_time branch from b8ba8c3 to 32d0747 Compare May 16, 2024 08:06
@arlyon arlyon changed the base branch from main to arlyon/turbo-trace-rustc-bug May 16, 2024 08:06
@arlyon arlyon force-pushed the arlyon/turbo-trace-rustc-bug branch from b90554a to f61c545 Compare May 16, 2024 08:09
@arlyon arlyon force-pushed the arlyon/turbo-trace-rustc-bug branch from f61c545 to b17a90e Compare May 16, 2024 11:00
@arlyon arlyon force-pushed the 03-08-add_additional_filter_modes_for_allocation_and_alloc_count_over_time branch from 32d0747 to 4c292ac Compare May 16, 2024 11:00
@arlyon arlyon force-pushed the arlyon/turbo-trace-rustc-bug branch 2 times, most recently from cf109b4 to fca37b6 Compare May 21, 2024 13:22
Base automatically changed from arlyon/turbo-trace-rustc-bug to main May 21, 2024 13:43
@arlyon arlyon force-pushed the 03-08-add_additional_filter_modes_for_allocation_and_alloc_count_over_time branch from 4c292ac to 19d73f2 Compare May 22, 2024 07:37
@arlyon arlyon merged commit 99d5ded into main May 22, 2024
54 of 56 checks passed
@arlyon arlyon deleted the 03-08-add_additional_filter_modes_for_allocation_and_alloc_count_over_time branch May 22, 2024 11:11
timneutkens added a commit to vercel/next.js that referenced this pull request May 26, 2024
## What?

Update: Implemented a feature in Turbopack for `typeof window` inlining:
vercel/turborepo#8211. Verified the changes in that
PR + this PR fix the test 💯


Turbopack upgrade:

* vercel/turborepo#8081 <!-- Alexander Lyon - add
support for upload speed / remaining in the cache upload step -->
* vercel/turborepo#7673 <!-- Alexander Lyon -
[turbo-trace] add additional filter modes for allocs over time -->
* vercel/turborepo#8191 <!-- Will Binns-Smith -
Turbopack: Register react refresh exports in module factory -->
* vercel/turborepo#8195 <!-- Donny/강동윤 - feat:
Remove lint for `grid-template-areas`" -->
* vercel/turborepo#8207 <!-- Benjamin Woodruff -
chore: Remove unused `base16` dependency and dead `hex` wrapper function
-->
* vercel/turborepo#8185 <!-- Tim Neutkens - Update
contributing guide to mention snapshot test updating -->
* vercel/turborepo#8211 <!-- Tim Neutkens - Add
option for inlining typeof window -->
* vercel/turborepo#8214 <!-- Will Binns-Smith -
Revert "Turbopack: Register react refresh exports in module factory
(#8191)" -->

---

Previous PR description::

Implements a failing test for issue #66058.
The test only fails when using Turbopack.

While digging into that issue I found that `typeof window` replacement
is either not applied or does not cause `import()` or `require()` to be
removed. In the reproduction that approach is used to load a particular
package only in Node.js and another only in the browser.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
…l/turborepo#7673)

This adds a few modes to see allocations per unit time. The viewer is
a bit borked but by selecting the one you want to inspect and changing the
view for sub-items you can usually find some useful data.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…l/turborepo#7673)

This adds a few modes to see allocations per unit time. The viewer is
a bit borked but by selecting the one you want to inspect and changing the
view for sub-items you can usually find some useful data.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…l/turborepo#7673)

This adds a few modes to see allocations per unit time. The viewer is
a bit borked but by selecting the one you want to inspect and changing the
view for sub-items you can usually find some useful data.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
…l/turborepo#7673)

This adds a few modes to see allocations per unit time. The viewer is
a bit borked but by selecting the one you want to inspect and changing the
view for sub-items you can usually find some useful data.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 14, 2024
Update: Implemented a feature in Turbopack for `typeof window` inlining:
vercel/turborepo#8211. Verified the changes in that
PR + this PR fix the test 💯

Turbopack upgrade:

* vercel/turborepo#8081 <!-- Alexander Lyon - add
support for upload speed / remaining in the cache upload step -->
* vercel/turborepo#7673 <!-- Alexander Lyon -
[turbo-trace] add additional filter modes for allocs over time -->
* vercel/turborepo#8191 <!-- Will Binns-Smith -
Turbopack: Register react refresh exports in module factory -->
* vercel/turborepo#8195 <!-- Donny/강동윤 - feat:
Remove lint for `grid-template-areas`" -->
* vercel/turborepo#8207 <!-- Benjamin Woodruff -
chore: Remove unused `base16` dependency and dead `hex` wrapper function
-->
* vercel/turborepo#8185 <!-- Tim Neutkens - Update
contributing guide to mention snapshot test updating -->
* vercel/turborepo#8211 <!-- Tim Neutkens - Add
option for inlining typeof window -->
* vercel/turborepo#8214 <!-- Will Binns-Smith -
Revert "Turbopack: Register react refresh exports in module factory
(#8191)" -->

---

Previous PR description::

Implements a failing test for issue #66058.
The test only fails when using Turbopack.

While digging into that issue I found that `typeof window` replacement
is either not applied or does not cause `import()` or `require()` to be
removed. In the reproduction that approach is used to load a particular
package only in Node.js and another only in the browser.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

Closes NEXT-
Fixes #

-->
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 15, 2024
Update: Implemented a feature in Turbopack for `typeof window` inlining:
vercel/turborepo#8211. Verified the changes in that
PR + this PR fix the test 💯

Turbopack upgrade:

* vercel/turborepo#8081 <!-- Alexander Lyon - add
support for upload speed / remaining in the cache upload step -->
* vercel/turborepo#7673 <!-- Alexander Lyon -
[turbo-trace] add additional filter modes for allocs over time -->
* vercel/turborepo#8191 <!-- Will Binns-Smith -
Turbopack: Register react refresh exports in module factory -->
* vercel/turborepo#8195 <!-- Donny/강동윤 - feat:
Remove lint for `grid-template-areas`" -->
* vercel/turborepo#8207 <!-- Benjamin Woodruff -
chore: Remove unused `base16` dependency and dead `hex` wrapper function
-->
* vercel/turborepo#8185 <!-- Tim Neutkens - Update
contributing guide to mention snapshot test updating -->
* vercel/turborepo#8211 <!-- Tim Neutkens - Add
option for inlining typeof window -->
* vercel/turborepo#8214 <!-- Will Binns-Smith -
Revert "Turbopack: Register react refresh exports in module factory
(#8191)" -->

---

Previous PR description::

Implements a failing test for issue #66058.
The test only fails when using Turbopack.

While digging into that issue I found that `typeof window` replacement
is either not applied or does not cause `import()` or `require()` to be
removed. In the reproduction that approach is used to load a particular
package only in Node.js and another only in the browser.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

Closes NEXT-
Fixes #

-->
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 16, 2024
## What?

Update: Implemented a feature in Turbopack for `typeof window` inlining:
vercel/turborepo#8211. Verified the changes in that
PR + this PR fix the test 💯


Turbopack upgrade:

* vercel/turborepo#8081 <!-- Alexander Lyon - add
support for upload speed / remaining in the cache upload step -->
* vercel/turborepo#7673 <!-- Alexander Lyon -
[turbo-trace] add additional filter modes for allocs over time -->
* vercel/turborepo#8191 <!-- Will Binns-Smith -
Turbopack: Register react refresh exports in module factory -->
* vercel/turborepo#8195 <!-- Donny/강동윤 - feat:
Remove lint for `grid-template-areas`" -->
* vercel/turborepo#8207 <!-- Benjamin Woodruff -
chore: Remove unused `base16` dependency and dead `hex` wrapper function
-->
* vercel/turborepo#8185 <!-- Tim Neutkens - Update
contributing guide to mention snapshot test updating -->
* vercel/turborepo#8211 <!-- Tim Neutkens - Add
option for inlining typeof window -->
* vercel/turborepo#8214 <!-- Will Binns-Smith -
Revert "Turbopack: Register react refresh exports in module factory
(#8191)" -->

---

Previous PR description::

Implements a failing test for issue #66058.
The test only fails when using Turbopack.

While digging into that issue I found that `typeof window` replacement
is either not applied or does not cause `import()` or `require()` to be
removed. In the reproduction that approach is used to load a particular
package only in Node.js and another only in the browser.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants