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

More robust vdbg! macro #4995

Merged
merged 1 commit into from
May 22, 2023
Merged

More robust vdbg! macro #4995

merged 1 commit into from
May 22, 2023

Conversation

alexkirsz
Copy link
Contributor

Description

I ran into a couple of issues with the vdbg! macro:

  • The macro tries to move its parameters within an async block, which requires everything that's moved to be both Copy and 'static to work properly. This would break when trying to pass &obj.property, where the reference doesn't have a 'static lifetime. Instead, I'm adding the requirement on parameters to impl ToOwned so we can ensure the 'static lifetime, and creating an owned version of the parameter to move into the future.
  • The CELL_COUNTERS thread local could be unset when calling .value_debug_format().try_to_string(), which would panic the thread.

@alexkirsz alexkirsz requested a review from a team as a code owner May 17, 2023 12:11
@vercel
Copy link

vercel bot commented May 17, 2023

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

Name Status Preview Updated (UTC)
turbo-site ✅ Ready (Inspect) Visit Preview May 17, 2023 0:11am
10 Ignored Deployments
Name Status Preview Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) May 17, 2023 0:11am
examples-cra-web ⬜️ Ignored (Inspect) May 17, 2023 0:11am
examples-designsystem-docs ⬜️ Ignored (Inspect) May 17, 2023 0:11am
examples-gatsby-web ⬜️ Ignored (Inspect) May 17, 2023 0:11am
examples-kitchensink-blog ⬜️ Ignored (Inspect) May 17, 2023 0:11am
examples-native-web ⬜️ Ignored (Inspect) May 17, 2023 0:11am
examples-nonmonorepo ⬜️ Ignored (Inspect) May 17, 2023 0:11am
examples-svelte-web ⬜️ Ignored (Inspect) May 17, 2023 0:11am
examples-tailwind-web ⬜️ Ignored (Inspect) May 17, 2023 0:11am
examples-vite-web ⬜️ Ignored (Inspect) May 17, 2023 0:11am

@github-actions
Copy link
Contributor

✅ This change can build next-swc

@github-actions
Copy link
Contributor

github-actions bot commented May 17, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

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

See workflow summary for details

@github-actions
Copy link
Contributor

Linux Benchmark for c5e4b31

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 4794.97µs ± 47.40µs 4815.44µs ± 35.95µs +0.43%
bench_hmr_to_eval/Turbopack CSR/1000 modules 4409.06µs ± 37.99µs 4788.20µs ± 531.40µs +8.60%
bench_startup/Turbopack CSR/1000 modules 712.40ms ± 1.57ms 729.37ms ± 8.05ms +2.38%

@github-actions
Copy link
Contributor

MacOS Benchmark for c5e4b31

Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 22.52ms ± 1.46ms 26.18ms ± 0.21ms +16.25% +1.24%
bench_hmr_to_eval/Turbopack CSR/1000 modules 25.04ms ± 0.73ms 46.76ms ± 6.02ms +86.74% +30.96%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 22.52ms ± 1.46ms 26.18ms ± 0.21ms +16.25% +1.24%
bench_hmr_to_eval/Turbopack CSR/1000 modules 25.04ms ± 0.73ms 46.76ms ± 6.02ms +86.74% +30.96%
bench_startup/Turbopack CSR/1000 modules 2836.26ms ± 65.93ms 2831.23ms ± 76.57ms -0.18%

@alexkirsz alexkirsz merged commit 40e6c17 into main May 22, 2023
@alexkirsz alexkirsz deleted the alexkirsz/vdbg-clone branch May 22, 2023 07:41
kodiakhq bot pushed a commit to vercel/next.js that referenced this pull request May 23, 2023
### What?

adds `NEXT_TURBOPACK_TRACING` env var to enable tracing. Writes into `.next/trace.log`

There are 4 presets:

* `NEXT_TURBOPACK_TRACING=overview` gives a overview of requests and modules processed.
* `NEXT_TURBOPACK_TRACING=next` above plus details for next.js
* `NEXT_TURBOPACK_TRACING=turbopack` above plus details for turbopack
* `NEXT_TURBOPACK_TRACING=turbo-tasks` above plus details for turbo-tasks

Published release builds will only allow `overview` to work, since all detailed instrumentation is statically disabled.

see vercel/turborepo#4966 for more details

### Why?

get more insight into build times

### Turbopack changes

* vercel/turborepo#4995 
* vercel/turborepo#5049 
* vercel/turborepo#5053 
* vercel/turborepo#4966
alexkirsz pushed a commit that referenced this pull request May 31, 2023
### Description

Two small fixes:

- The `vdbg!(vc; depth = 1)` macro would error out
  - the expanded `__init` syntax wasn't updated in #4995
- The JSON error message could panic if the line and/or column was `0`
- This comes up when trying to deserialize into a struct with a missing
prop, because it errors at the opening `{` brace

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->
hydRAnger pushed a commit to hydRAnger/next.js that referenced this pull request Jun 12, 2023
### What?

adds `NEXT_TURBOPACK_TRACING` env var to enable tracing. Writes into `.next/trace.log`

There are 4 presets:

* `NEXT_TURBOPACK_TRACING=overview` gives a overview of requests and modules processed.
* `NEXT_TURBOPACK_TRACING=next` above plus details for next.js
* `NEXT_TURBOPACK_TRACING=turbopack` above plus details for turbopack
* `NEXT_TURBOPACK_TRACING=turbo-tasks` above plus details for turbo-tasks

Published release builds will only allow `overview` to work, since all detailed instrumentation is statically disabled.

see vercel/turborepo#4966 for more details

### Why?

get more insight into build times

### Turbopack changes

* vercel/turborepo#4995 
* vercel/turborepo#5049 
* vercel/turborepo#5053 
* vercel/turborepo#4966
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
### Description

I ran into a couple of issues with the vdbg! macro:
* The macro tries to move its parameters within an async block, which
requires everything that's moved to be both `Copy` and `'static` to work
properly. This would break when trying to pass `&obj.property`, where
the reference doesn't have a `'static` lifetime. Instead, I'm adding the
requirement on parameters to impl `ToOwned` so we can ensure the
`'static` lifetime, and creating an owned version of the parameter to
move into the future.
* The `CELL_COUNTERS` thread local could be unset when calling
`.value_debug_format().try_to_string()`, which would panic the thread.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
### Description

Two small fixes:

- The `vdbg!(vc; depth = 1)` macro would error out
  - the expanded `__init` syntax wasn't updated in vercel/turborepo#4995
- The JSON error message could panic if the line and/or column was `0`
- This comes up when trying to deserialize into a struct with a missing
prop, because it errors at the opening `{` brace

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
### Description

I ran into a couple of issues with the vdbg! macro:
* The macro tries to move its parameters within an async block, which
requires everything that's moved to be both `Copy` and `'static` to work
properly. This would break when trying to pass `&obj.property`, where
the reference doesn't have a `'static` lifetime. Instead, I'm adding the
requirement on parameters to impl `ToOwned` so we can ensure the
`'static` lifetime, and creating an owned version of the parameter to
move into the future.
* The `CELL_COUNTERS` thread local could be unset when calling
`.value_debug_format().try_to_string()`, which would panic the thread.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
### Description

Two small fixes:

- The `vdbg!(vc; depth = 1)` macro would error out
  - the expanded `__init` syntax wasn't updated in vercel/turborepo#4995
- The JSON error message could panic if the line and/or column was `0`
- This comes up when trying to deserialize into a struct with a missing
prop, because it errors at the opening `{` brace

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
### Description

I ran into a couple of issues with the vdbg! macro:
* The macro tries to move its parameters within an async block, which
requires everything that's moved to be both `Copy` and `'static` to work
properly. This would break when trying to pass `&obj.property`, where
the reference doesn't have a `'static` lifetime. Instead, I'm adding the
requirement on parameters to impl `ToOwned` so we can ensure the
`'static` lifetime, and creating an owned version of the parameter to
move into the future.
* The `CELL_COUNTERS` thread local could be unset when calling
`.value_debug_format().try_to_string()`, which would panic the thread.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
### Description

Two small fixes:

- The `vdbg!(vc; depth = 1)` macro would error out
  - the expanded `__init` syntax wasn't updated in vercel/turborepo#4995
- The JSON error message could panic if the line and/or column was `0`
- This comes up when trying to deserialize into a struct with a missing
prop, because it errors at the opening `{` brace

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants