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

build: don't use Nx cache #1186

Merged
merged 1 commit into from Jan 3, 2023
Merged

build: don't use Nx cache #1186

merged 1 commit into from Jan 3, 2023

Conversation

samestep
Copy link
Collaborator

@samestep samestep commented Jan 3, 2023

Description

This PR removes the monorepo build artifact caching added by #1081 and #1142, also removing the need for #1175. In practice, my observation has been that most of our tasks are either very fast (e.g. esbuild) or already cache automatically (e.g. Cargo), and that for the ones that aren't super fast (e.g. Vite build in components or editor), Nx usually doesn't cache those anyway because their dependencies changed. Furthermore, it requires a lot of configuration to correctly identify cache hits; so far we've only been specifying the outputs of each task and not the inputs, so Nx usually reruns tasks unnecessarily because it sees changes in the output artifacts of downstream tasks and conservatively assumes that those might affect things. On the flipside, I've also often found myself running rm -r node_modules/.cache/ just to be sure that Nx isn't caching things incorrectly. Specifically, some of our scripts (e.g. wasm-bindgen) don't delete the contents of their output directory before writing to it, so if they aren't writing as many files as they were before, Nx will still pick those up and put them in the cache even though they aren't outputs of the current task run. In theory we could deal with this by manually deleting these output directories before writing to them, but even that doesn't really seem feasible: for instance, if we were to always delete target/ before running Cargo, that would get rid of its own caching abilities, which wouldn't be worth it.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new ESLint warnings
  • I have reviewed any generated registry diagram changes

@samestep samestep requested a review from wodeni January 3, 2023 20:22
@github-actions
Copy link

github-actions bot commented Jan 3, 2023

± Registry diff

📊 Performance

Key

Note that each bar component rounds up to the nearest 100ms, so each full bar is an overestimate by up to 400ms.

     0s   1s   2s   3s   4s   5s   6s   7s   8s   9s
     |    |    |    |    |    |    |    |    |    |
name ▝▀▀▀▀▀▀▀▀▀▀▀▚▄▄▄▄▄▄▄▄▄▞▀▀▀▀▀▀▀▀▀▀▀▀▚▄▄▄▄▄▄▄▄▄▖
      compilation labelling optimization rendering

Data

                                        0s   1s   2s   3s   4s   5s   6s   7s   8s   9s
                                        |    |    |    |    |    |    |    |    |    |
3d-projection-fake-3d-linear-algebra    ▝▀▚▚
allShapes-allShapes                     ▝▀▀▀▚▞▄▄
arrowheads-arrowheads                   ▝▀▚▚
circle-example-euclidean                ▝▀▀▀▀▀▀▄▀▀▀▚
collinear-euclidean                     ▝▀▀▀▀▚▚
congruent-triangles-euclidean           ▝▀▀▀▀▀▀▀▀▀▚▚
continuousmap-continuousmap             ▝▀▚▚
hypergraph-hypergraph                   ▝▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▞▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▖
incenter-triangle-euclidean             ▝▀▀▀▀▀▀▚▚
lagrange-bases-lagrange-bases           ▝▀▀▚▚
midsegment-triangles-euclidean          ▝▀▀▀▀▞▖
non-convex-non-convex                   ▝▀▀▀▀▀▞▖
one-water-molecule-atoms-and-bonds      ▝▚▚
parallel-lines-euclidean                ▝▀▀▀▀▀▀▞▖
persistent-homology-persistent-homology ▝▀▀▀▀▀▀▀▀▞▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▚▖
points-around-line-shape-distance       ▝▀▀▀▀▚▚
points-around-polyline-shape-distance   ▝▀▀▀▀▀▀▀▀▀▀▀▀▀▀▞▖
points-around-star-shape-distance       ▝▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▚▚
siggraph-teaser-euclidean-teaser        ▝▀▀▀▀▀▚▚
small-graph-disjoint-rect-line-horiz    ▝▀▀▀▀▀▀▀▀▀▀▀▀▀▞▖
small-graph-disjoint-rects              ▝▀▚▚
small-graph-disjoint-rects-large-canvas ▝▀▀▞▖
small-graph-disjoint-rects-small-canvas ▝▀▀▞▖
tree-tree                               ▝▀▀▚▄▄▞▖
tree-venn                               ▝▀▀▀▀▀▄▚
tree-venn-3d                            ▝▀▀▀▚▚▄
two-vectors-perp-vectors-dashed         ▝▀▀▞▖
vector-wedge-exterior-algebra           ▝▀▀▞▖
wet-floor-atoms-and-bonds               ▝▀▀▀▞▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▖
word-cloud-example-word-cloud           ▝▀▀▀▀▀▀▞▖
wos-laplace-estimator-walk-on-spheres   ▝▀▀▀▀▀▀▀▀▀▀▞▖
wos-nested-estimator-walk-on-spheres    ▝▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▚▀▀▀▀▀▀▀▀▀▖
wos-offcenter-estimator-walk-on-spheres ▝▀▀▀▀▀▀▀▀▀▀▀▚▚
wos-poisson-estimator-walk-on-spheres   ▝▀▀▀▀▀▀▀▀▀▀▀▀▚▀▀▚

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Merging #1186 (587cb00) into main (c2c020d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1186   +/-   ##
=======================================
  Coverage   62.91%   62.91%           
=======================================
  Files          59       59           
  Lines        7164     7164           
  Branches     1673     1673           
=======================================
  Hits         4507     4507           
  Misses       2572     2572           
  Partials       85       85           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@samestep samestep merged commit 4b435b4 into main Jan 3, 2023
@samestep samestep deleted the no-nx-cache branch January 3, 2023 20:38
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.

None yet

2 participants