Defer creation of cache directory

[mitchhentgesspotify]

We've been redundantly telling the system to re-create the cache directory for transformed files, even when just looking up already-transformed ones.
This syscall has a light impact when occurring in large codebases.

Improve this by deferring createDirectory until right before we save the "transform result".

[SimenB]

This makes sense to me 🙂 Can you fix CI?

[mitchhentgesspotify]

Hey, I'm currently fighting with CI and bisecting down to why a failure is occurring. I'll bump to "Ready for review" when this one is good to go :)

[SimenB]

CI doesn't trigger on conflicts, which is why I resolved it 🙂

[SimenB]

Only failure on last run was a weird installation error on windows, so I think this was ready to go up until the force push 😅

https://github.com/facebook/jest/actions/runs/3273586906/jobs/5386009846

[mitchhentgesspotify]

Only failure on last run was a weird installation error on windows, so I think this was ready to go up until the force push sweat_smile

Ah, that's because my current DEV CI has some extra code doing an extra createDirectory(), which I'm slowly shuffling down to find out where it's needed. Without that "redundant" call, CI is failing.

CI doesn't trigger on conflicts, which is why I resolved it slightly_smiling_face

Aha, I see. Thanks! I'll make sure to rebase for further testing 👍

[cpojer]

Can you quantify the impact you are seeing through this syscall with and without this change? I'd like to see some benchmarks.

What if instead we update createDirectory to stat the path first before creating the directory?

[mitchhentgesspotify]

Can you quantify the impact you are seeing through this syscall with and without this change? I'd like to see some benchmarks.

It's not major, it was tough to measure with "performance jitter". However, it's a clear theoretical optimization (skip an operation that would touch the FS, which is generally a "relatively" slow operation).

What if instead we update createDirectory to stat the path first before creating the directory?

We'd be trading 1 syscall for 1-or-2 syscalls.

---

If the PR/CI churn is too noisy, then I can drop this change, as it's not majorly significant.

[mitchhentgesspotify]

Hmm, this ENOENT: no such file or directory, open '/tmp/jest_rt/jest-transform-cache-9e806c21c042d907aa515543cd8b5996-af28b5a988839dd05b996f331ba8234d/33/test_33a6cab28265dd8aa42236556dd3e4b2.450646703' error is tricky to track down, and behaves different based on where I move a redundant createDirectory() call. It doesn't help that I've had to defer this work and come back to it with less context each time, but it smells like there's some magic involved causing the bad behaviour.

I'll give this another couple passes, but I'm almost ready to concede defeat based on the work/reward ratio here 😅

[mitchhentgesspotify]

Fixed now, there was an incorrect variable used, but it was hard for me to track down because I couldn't repro the failure locally.
Should be ready for review now, while being a relatively simple patch (now that it's green 😅)

[SimenB]

cool, thanks 👍

[mitchhentgesspotify]

Sorry for the long wait, and thanks for the fast response. Cheers Simen 🍻

[SimenB]

https://github.com/facebook/jest/releases/tag/v29.3.1

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.