Use vm.Script and refactor coverage caching.

[cpojer]

This diff:

• uses vm.Script instead of strings for executing code in contexts.
• Removes the unused CoverageCollector class.
• Refactors IstanbulCollector to be more sane.
• Now properly caches files when using jest --coverage. We now no longer transform files more than once when they are unchanged and coverage is used.

[cpojer]

To further improve performance I tried to use vm.Script's cachedData option that allows to store and read bytecode: https://gist.github.com/cpojer/fea3636b94406f56490716270d61d899

However, it yields no performance benefits that I can identify. I ran Jest twice, once writing the bytecode and once with reading from cache. I then compared this PR with the code in the gist above and ran Jest multiple times. I could not identify a statistically relevant performance difference when using this feature, unfortunately. I thought that parsing and compiling a script would take up a relevant portion of the runtime but it doesn't seem relevant.

It seems like the bytecode cache is about 2-3 times smaller than the source text, which would be a nice win for memory and cache size. However, a vm.Script instance requires the original source text as well as the cached bytecode. It seems like we should build a vm.CachedScript(buffer, options?) feature into node but overall there seems little value other than memory use. cc @indutny what do you think about that?

See nodejs/node@96934cb and https://nodejs.org/api/vm.html#vm_class_vm_script for the node.js feature.

The rest of this PR is still relevant, of course.

[indutny]

@cpojer does it set cachedDataRejected to false when you pass the cached data in?

[cpojer]

cachedDataProduced is false, yes, so everything appears to be working correctly. I only tested on a small size of files, however, which might have something to do with this. What do you think about the chances of adding vm.CompiledScript to node? It would take a buffer without the original source code and would leave the validation up to the user (in our case we'd simply check whether the source file has changed or not). I'd be happy to attempt to make a PR.

[cpojer]

I just tested it on a larger number of files (453) and the difference seems to be less than 10ms. Is this expected? Is parsing and compiling so fast these days that everything else just dwarfs it?

[indutny]

@cpojer what about cachedDataRejected?

I don't think that there is a huge difference when re-using cached data for small files.

[cpojer]

Adding console.log(script.cachedDataRejected, script.cachedDataProduced); will tell me that it wasn't rejected (false) and that it didn't produce new cache data (undefined).

Are you saying this feature is not very useful for many small files but would be more useful for a few big files? Is there anything we can tweak to make this more useful?

[indutny]

@cpojer ok, good to know! Thank you!

Honestly saying, I don't know it for sure. From my local test the require got boosted a bit:

nodejs/node#4777

[aaronabramov]

what does this line do?

[cpojer]

Basically right now we create one instrumenter per test file, so this just makes sure that we put the coverage info into the $JEST variable so we can pull it out later.

I'm sure you'll rewrite and fix this soon, however :)

[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.