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

feat(turbopack-ecmascript): implement acyclic SCC graph for ESM imports #5506

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

ForsakenHarmony
Copy link
Contributor

Description

We compute a graph of strongly connected components of all ESM imports in a chunk group.
Computing all SCCs in a graph guarantees that the graph of SCCs is acyclic as any cycle is contained in the strongly connected component.

This should help the async modules implementation by allowing it to not worry about import cycles.

@ForsakenHarmony ForsakenHarmony requested a review from a team as a code owner July 12, 2023 19:18
@vercel
Copy link

vercel bot commented Jul 12, 2023

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

Name Status Preview Comments Updated (UTC)
examples-cra-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 18, 2023 1:14pm
examples-tailwind-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 18, 2023 1:14pm
examples-vite-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Jul 18, 2023 1:14pm
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 18, 2023 1:14pm
7 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jul 18, 2023 1:14pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jul 18, 2023 1:14pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jul 18, 2023 1:14pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Jul 18, 2023 1:14pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Jul 18, 2023 1:14pm
examples-nonmonorepo ⬜️ Ignored (Inspect) Visit Preview Jul 18, 2023 1:14pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jul 18, 2023 1:14pm

@github-actions
Copy link
Contributor

github-actions bot commented Jul 12, 2023

✅ This change can build next-swc

@ForsakenHarmony
Copy link
Contributor Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 12, 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 45cae2a

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 5455.81µs ± 20.77µs 5381.61µs ± 40.92µs -1.36%
bench_hmr_to_eval/Turbopack CSR/1000 modules 5082.98µs ± 33.08µs 5102.24µs ± 18.67µs +0.38%
bench_startup/Turbopack CSR/1000 modules 803.44ms ± 0.96ms 805.31ms ± 2.67ms +0.23%

Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

I like the new try_flat_join

@github-actions
Copy link
Contributor

Linux Benchmark for 830b5f1

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 7874.24µs ± 54.29µs 7786.60µs ± 78.49µs -1.11%
bench_hmr_to_eval/Turbopack CSR/1000 modules 5831.31µs ± 131.58µs 5517.52µs ± 195.87µs -5.38%
bench_startup/Turbopack CSR/1000 modules 821.11ms ± 2.24ms 820.87ms ± 1.14ms -0.03%

@github-actions
Copy link
Contributor

MacOS Benchmark for 830b5f1

Test Base PR % Significant %
bench_hmr_to_eval/Turbopack CSR/1000 modules 24.78ms ± 0.91ms 47.21ms ± 5.47ms +90.52% +36.37%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 27.90ms ± 0.20ms 26.89ms ± 0.74ms -3.60%
bench_hmr_to_eval/Turbopack CSR/1000 modules 24.78ms ± 0.91ms 47.21ms ± 5.47ms +90.52% +36.37%
bench_startup/Turbopack CSR/1000 modules 4360.87ms ± 837.11ms 3165.62ms ± 101.16ms -27.41%

@ForsakenHarmony ForsakenHarmony merged commit 814f0a2 into main Jul 18, 2023
@ForsakenHarmony ForsakenHarmony deleted the hrmny/ecmascript-scc-graph branch July 18, 2023 15:31
X-oss-byte added a commit to X-oss-byte/turbo that referenced this pull request Jul 18, 2023
sokra added a commit to vercel/next.js that referenced this pull request Jul 18, 2023
### What?

refactoring see vercel/turborepo#5557

### Turbopack Changes

* vercel/turborepo#5506 <!-- Leah -
feat(turbopack-ecmascript): implement acyclic SCC graph for ESM imports
-->
* vercel/turborepo#5557 <!-- Tobias Koppers - Ensure
output assets reference only output assets -->
NicholasLYang pushed a commit that referenced this pull request Jul 21, 2023
commit 6c178d2
Author: Alex Kirszenberg <alex.kirszenberg@vercel.com>
Date:   Thu Jul 20 18:06:03 2023 +0200

    Extract shared HMR utils to their own modules/crates (#5576)

    ### Description

    These will also be used in Next.js by the Nexturbo dev API.

    ### Testing Instructions

    N/A

commit e5f43a5
Author: Chris Olszewski <chris.olszewski@vercel.com>
Date:   Wed Jul 19 18:03:12 2023 -0700

    fix: pnpm alias workspace deps (#5569)

    ### Description

    Fixes #5441

    Adds support for [referencing workspaces through
    aliases](https://pnpm.io/workspaces#referencing-workspace-packages-through-aliases)
    by properly resolving them to the correct workspace. Before we would
    mark a package as being an external dependency (or if the alias was a
    valid workspace depend on the incorrect one).

    This PR now recognizes when `workspace:` dependency references a
    different package than the name that's used in the `package.json`.

    Note for reviewers:
    This probably isn't the cleanest solution in either Rust or Go, but
    while we need to maintain two codepaths this keeps the code roughly
    equivalent.

    ### Testing Instructions

    Added unit tests on the Go side.

    Tested manually with a repository where `web` specified it's dependency
    on `@scope/ui` as `"ui": "workspace:@scope/ui@*" and verified that:
    - `turbo run build`: `@scope/ui` finished building before building
    `web`, this hits the Go impl
    - `turbo prune --scope=web`: `@scope/ui` was included in the pruned
    repository, this hits the Rust impl

    ---------

    Co-authored-by: Chris Olszewski <Chris Olszewski>

commit 396bf45
Author: Chris Olszewski <chris.olszewski@vercel.com>
Date:   Wed Jul 19 13:31:35 2023 -0700

    feat: port prune to rust (#5531)

    ### Description

    Port prune from Go to Rust and remove the old Go implementation. To
    achieve this the following was done:
     - Parsing the lockfile during package graph construction
     - Porting utility functions
    - Some minor changes in `turborepo_paths`, these were primarily moving
    methods from their owned to borrowed counterparts
    - Expanding `package.json` parsing to grab some of the information
    pruning requires
    - Porting of the prune command itself. I added some additional structure
    compared to the Go version, but not enough that comparing it to the Go
    version should be difficult

    Notes for reviewers:
    I apologize that this PR ended up touching as much as it did. Reviewing
    the PR by commit should at least make all of the changes and their
    impacts obvious. Commits before `2d4154c` are already on main and can be
    skipped.

    ### Testing Instructions

    Existing unit tests and integration tests for file copying and package
    graph traversal. Actual lockfile behavior is mostly covered by unit
    tests that were ported when the lockfile were ported.

    Also did various manual testing with pruning monorepos.

    ---------

    Co-authored-by: Chris Olszewski <Chris Olszewski>

commit 007b574
Author: Alex Kirszenberg <alex.kirszenberg@vercel.com>
Date:   Wed Jul 19 22:17:39 2023 +0200

    Remove unnecessary ValueDebugFormat item, hide Vc field (#5567)

    ### Description

    This removes unnecessary items from the IDE's autosuggestion.

    I'd also like to get rid of all the `*_inline` suggestions, but RA will
    ignore `#[doc(hidden)]` when inside the same crate. I don't think
    there's a way to indicate "never, ever suggest this item". We could move
    some of these to be private to some generated module, but that module
    would probably still show up as a top level suggestion.

    ### Testing Instructions

    Automated tests.

commit d13b812
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Wed Jul 19 11:32:07 2023 -0700

    release(turborepo): 1.10.9 (#5565)

    Co-authored-by: Turbobot <turbobot@vercel.com>
    Co-authored-by: Chris Olszewski <Chris Olszewski>

commit 229a2a4
Author: Tobias Koppers <tobias.koppers@googlemail.com>
Date:   Wed Jul 19 16:08:16 2023 +0200

    make with_layer return Vc<Self> (#5563)

    ### Description

    Using the new ability to return `Vc<Self>`

    Co-authored-by: Alex Kirszenberg <alexkirsz@users.noreply.github.com>

commit c78593b
Author: Greg Soltis <greg.soltis@vercel.com>
Date:   Tue Jul 18 20:11:16 2023 -0700

    feat(turborepo): Add proxy support to create-turbo and turbo-gen (#5554)

    Co-authored-by: Greg Soltis <Greg Soltis>

commit 5ab8ac0
Author: Chris Olszewski <chris.olszewski@vercel.com>
Date:   Tue Jul 18 15:06:38 2023 -0700

    fix(lockfile): Fix directory resolution variant (#5551)

    ### Description

    Fixes #5529

    During the Rust migration I must've messed up the directory field name.
    Double checked against
    [`@pnpm/lockfile-types`](https://github.com/pnpm/pnpm/blob/main/lockfile/lockfile-types/src/index.ts#L86)
    to make sure all of the fields are correct now.

    `PackageResolution` should be an enum, but the fact that tarballs are an
    untagged variant makes that tricky to communicate to `serde`. A struct
    does enough for us.

    ### Testing Instructions

    Added new unit test to make sure we don't lose any fields for the
    various variants of the `resolution` field

    ---------

    Co-authored-by: Chris Olszewski <Chris Olszewski>

commit ca7e3e4
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Tue Jul 18 14:51:34 2023 -0700

    release(turborepo): 1.10.9-canary.0 (#5559)

    Co-authored-by: Turbobot <turbobot@vercel.com>

commit fc5e2b0
Author: Chris Olszewski <chris.olszewski@vercel.com>
Date:   Tue Jul 18 13:16:16 2023 -0700

    chore: no longer convert in relative unix path constructor (#5552)

    ### Description

    To quote @gsoltis:
    > In general:
    > - constructors should validate, to the extent they can (not much for
    `unix` paths, but can verify relative)
    > - conversions should be explicit. You need to know where you're
    starting from. If this were an AnchoredSystemPath on windows, the `\` ->
    `/` makes sense. If it's a literal from e.g. a tar file, then it
    doesn't.

    Reviewers Notes:
    - Opening up this PR in VSCode and using `Find All References` on the
    constructor is useful for double checking that I didn't miss a
    conversion.
    - Clippy error appeared on local when I made these changes. Fixed it
    just in case that would block CI
    - Moved `to_unix` to `AnchoredSystemPath` instead of
    `AnchoredSystemPathBuf` now that we have deref coercion which will
    automatically convert `&AnchroedSystemPathBuf` to `&AnchoredSystemPath`
    and moving the method allows it to be called from either type.

    ### Testing Instructions

    Looked through all uses of `RelativeUnixPathBuf::new` to see if there
    were places that depended on the conversion. The only use that was
    obvious was the usage in `cache_archive/create.rs`. `dotEnv` was the
    only other place where we possibly were converting a system path to a
    relative unix. We don't specify that `dotEnv` entries should be unix
    relative, so we might've been accidentally supporting system paths, but

    ---------

    Co-authored-by: Chris Olszewski <Chris Olszewski>

commit 3eb3a5f
Author: Tobias Koppers <tobias.koppers@googlemail.com>
Date:   Tue Jul 18 21:21:51 2023 +0200

    Ensure output assets reference only output assets (#5557)

    ### Description

    Ensure output assets reference only output assets

    next.js PR: vercel/next.js#52832

commit 22f0bf4
Author: Nathan Hammond <nathan.hammond@vercel.com>
Date:   Wed Jul 19 02:59:12 2023 +0800

    Remove binary optimization step. (#5543)

    Co-authored-by: Nathan Hammond <Nathan Hammond>

commit 5955625
Author: mknichel <7355009+mknichel@users.noreply.github.com>
Date:   Tue Jul 18 10:09:28 2023 -0700

    fix(turborepo): Allow users to select a Vercel team when linking a repository to a Space (#5533)

commit 814f0a2
Author: Leah <github.leah@hrmny.sh>
Date:   Tue Jul 18 17:31:33 2023 +0200

    feat(turbopack-ecmascript): implement acyclic SCC graph for ESM imports (#5506)

commit 46bb9b7
Author: Tobias Koppers <tobias.koppers@googlemail.com>
Date:   Tue Jul 18 13:41:01 2023 +0200

    move references() to specific traits (#5555)

    ### Description

    preparation for making different typed references

    next.js PR: vercel/next.js#52822

commit 56edd9e
Author: Greg Soltis <greg.soltis@vercel.com>
Date:   Mon Jul 17 21:37:26 2023 -0700

    feat(turborepo): Add task ids to failure reports (#5535)

    Co-authored-by: Greg Soltis <Greg Soltis>

commit ea934d1
Author: Greg Soltis <greg.soltis@vercel.com>
Date:   Mon Jul 17 13:52:59 2023 -0700

    chore(turborepo): Turborepo owns the examples-tests directory (#5540)

    Co-authored-by: Greg Soltis <Greg Soltis>

commit 5279282
Author: Alex Kirszenberg <alex.kirszenberg@vercel.com>
Date:   Mon Jul 17 19:55:22 2023 +0200

    Add missing feature to syn (#5547)

    ### Description

    Cargo check currently fails with:

    ```
    error[E0277]: `syn::Type` doesn't implement `Debug`
     --> crates/turbo-tasks-macros-shared/src/primitive_input.rs:8:5
      |
    6 | #[derive(Debug)]
      |          ----- in this derive macro expansion
    7 | pub struct PrimitiveInput {
    8 |     pub ty: Type,
      |     ^^^^^^^^^^^^ `syn::Type` cannot be formatted using `{:?}` because it doesn't implement `Debug`
      |
      = help: the trait `Debug` is not implemented for `syn::Type`
      = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)

    For more information about this error, try `rustc --explain E0277`.
    error: could not compile `turbo-tasks-macros-shared` (lib) due to previous error
    ```

    This is because because turbo-tasks-macros-shared (which is missing a
    syn feature to enable the debug trait) is part of the workspace default
    members, while turbo-tasks-macros (which has the feature) isn’t.

    ### Testing Instructions

    `cargo check`

commit 78f6cc8
Author: Chris Olszewski <chris.olszewski@vercel.com>
Date:   Mon Jul 17 10:40:13 2023 -0700

    docs: document log order (#5463)

    ### Description

    Adds documentation for `--log-order` #3916 including a callout of our
    special behavior on Github Actions.

    ### Testing Instructions

    Eyes

    ---------

    Co-authored-by: Chris Olszewski <Chris Olszewski>

commit 0771b80
Author: Amit Gurbani <amit1994.gurbani@gmail.com>
Date:   Mon Jul 17 23:09:44 2023 +0530

    Update storybook.mdx (#5537)

    ### Description

    Storybook now can be built with node 18, hence removing this from
    documentation.

    Co-authored-by: Anthony Shew <anthony.shew@vercel.com>

commit 91ca2ae
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Mon Jul 17 10:11:06 2023 -0700

    release(turborepo): 1.10.8 (#5546)

    Co-authored-by: Turbobot <turbobot@vercel.com>

commit d24075f
Author: Greg Soltis <greg.soltis@vercel.com>
Date:   Mon Jul 17 08:12:38 2023 -0700

    fix(turborepo): Set corepack dir for examples (#5539)

    ### Description

    - set a corepack install directory per example to avoid concurrency
    issues
     - set the `PATH` for the test to include the corepack directory

    ### Testing Instructions

    Examples tests

    ---------

    Co-authored-by: Greg Soltis <Greg Soltis>

commit 46d0945
Author: Nicholas Yang <nicholas.yang@vercel.com>
Date:   Mon Jul 17 11:07:45 2023 -0400

    feat(turborepo): FS Cache (#5473)

    ### Description

    Implements the FS cache on top of CacheItem. ~~This is stacked on top of
    #5065~~

    ### Testing Instructions

    Uses the same round-trip tests of HTTP cache.

    ---------

    Co-authored-by: --global <Nicholas Yang>
    Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>

commit 135c08f
Author: Tobias Koppers <tobias.koppers@googlemail.com>
Date:   Mon Jul 17 16:44:56 2023 +0200

    add direct cycle detection (#5544)

    ### Description

    very simple detection of dumb mistakes

commit 02f55d9
Author: Tobias Koppers <tobias.koppers@googlemail.com>
Date:   Mon Jul 17 16:33:34 2023 +0200

    move Asset::ident to more specific traits (#5528)

    ### Description

    * `ident()` is no longer on Asset, but on `Module`, `Source`,
    `OutputAsset` or `Chunk`
    * On the way, more AssetVc types needed to be switched to more specific
    traits

    next.js PR: vercel/next.js#52683

commit b069545
Author: Alex Kirszenberg <alex.kirszenberg@vercel.com>
Date:   Mon Jul 17 13:41:18 2023 +0200

    AdjacencyMap::reverse_topological (+ fixes) (#5527)

    ### Description

    This adds `AdjacencyMap::reverse_topological`, which is similar to
    `AdjacencyMap::into_reverse_topological` but doesn't consume the graph.

    This also:
    * Makes `AdjacencyMap` storable in `turbo_tasks::value`s;
    * Fixes ValueDebugFormat and TraceRawVcs derive macros so they support
    generic argument and bounds properly.

    ### Testing Instructions

    N/A

commit 8433a32
Author: Alex Kirszenberg <alex.kirszenberg@vercel.com>
Date:   Sun Jul 16 14:07:49 2023 +0200

    Vc<T> and Turbo Engine type system improvements (#4587)

    This PR changes our Turbo Engine code generation from generating
    additional `TypeVc` types to allowing the `Vc<Type>` notation. It also
    brings other improvements to the Turbo Engine type system, like more
    type-safe downcasting and upcasting, better support for primitives, the
    possibility to accept `&self` in `#[value_impl]` implementations
    everywhere, and a bunch of other changes.

    link WEB-379

commit fdc358a
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Fri Jul 14 20:31:07 2023 -0700

    release(turborepo): 1.10.8-canary.2 (#5534)

    Co-authored-by: Turbobot <turbobot@vercel.com>

commit 26fee25
Author: Greg Soltis <greg.soltis@vercel.com>
Date:   Fri Jul 14 11:13:59 2023 -0700

    fix(turborepo): Export and match on our copy of BasicUI (#5532)

    Co-authored-by: Greg Soltis <Greg Soltis>

commit b0ea0a8
Author: Greg Soltis <greg.soltis@vercel.com>
Date:   Fri Jul 14 10:39:52 2023 -0700

    fix(turborepo): Rebuild turbo if Go code has changed (#5530)

    Co-authored-by: Greg Soltis <Greg Soltis>

commit b6bb8fe
Author: Chris Olszewski <chris.olszewski@vercel.com>
Date:   Fri Jul 14 06:47:13 2023 -0700

    chore: use fs-err in turborepo fs related libs (#5517)

    ### Description
    Swaps our usage of various `fs` methods to use `fs-err` instead. To
    quote the `fs-err` docs:

    > Using [std::fs](https://doc.rust-lang.org/stable/std/fs/), if this
    code fails:
    >
    > `let file = File::open("does not exist.txt")?;`
    >
    > The error message that Rust gives you isn't very useful:
    >
    > `The system cannot find the file specified. (os error 2)`
    >
    > ...but if we use `fs-err` instead, our error contains more actionable
    information:
    >
    > ```failed to open file `does not exist.txt`
    > caused by: The system cannot find the file specified. (os error 2)```

    ### Testing Instructions
    Existing unit tests pass

    Co-authored-by: Chris Olszewski <Chris Olszewski>

commit f3a36e7
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Thu Jul 13 15:27:21 2023 -0700

    release(turborepo): 1.10.8-canary.1 (#5523)

    Co-authored-by: Turbobot <turbobot@vercel.com>

commit cabca3c
Author: Nicholas Yang <nicholas.yang@vercel.com>
Date:   Thu Jul 13 16:12:46 2023 -0400

    chore: Extend examples timeout (#5504)

    ### Description

    We get a lot of timeout failures on examples tests. Since this almost
    always requires re-running the test, which adds another 30 minutes of
    compute time, it's probably better to extend the timeout to 40 minutes.

    ### Testing Instructions

    <!--
      Give a quick description of steps to test your changes.
    -->

    Co-authored-by: nicholaslyang <Nicholas Yang>

commit 7c18c80
Author: Greg Soltis <greg.soltis@vercel.com>
Date:   Thu Jul 13 12:58:14 2023 -0700

    Implement hashing fallback (#5505)

    Co-authored-by: Greg Soltis <Greg Soltis>

commit 67e71c7
Author: Anthony Shew <anthony.shew@vercel.com>
Date:   Thu Jul 13 11:35:08 2023 -0700

    Better spot for link. (#5520)

commit a14180e
Author: Anthony Shew <anthony.shew@vercel.com>
Date:   Thu Jul 13 11:24:26 2023 -0700

    Fix link. (#5518)

commit b7aaa7b
Author: Nicholas Yang <nicholas.yang@vercel.com>
Date:   Thu Jul 13 13:18:44 2023 -0400

    chore: Added clippy deny all to crates (#5514)

    Co-authored-by: nicholaslyang <Nicholas Yang>

commit e3c68fa
Author: Alex Kirszenberg <alex.kirszenberg@vercel.com>
Date:   Thu Jul 13 16:34:27 2023 +0200

    Add any_content_changed_of_output_assets (#5513)

    ### Description

    Required for vercel/next.js#52259

    ### Testing Instructions

    N/A

    Co-authored-by: Tobias Koppers <sokra@users.noreply.github.com>

commit 4022f2b
Author: Tobias Koppers <tobias.koppers@googlemail.com>
Date:   Thu Jul 13 14:54:55 2023 +0200

    OutputAsset trait (#5507)

    ### Description

    adding a trait to all assets in the output graph

    next.js PR: vercel/next.js#52606
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants