Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[perf] Improve ExportMap.for performance on larger codebases
While looking at a larger code base https://gitlab.com/gitlab-org/gitlab, `ExportMap.for` seems to take a significant amount of time. More than half of it is spend on calculating hashes with `hashObject`. Digging a little deeper, it seems like we are calling it around 500 thousand times. Each iteration calculates the hash of a context object. This context object is created inside of the `childContext` function and consists of four parts: - `settings` -> an Object itself - `parserOptions` -> an Object itself - `parserPath` -> a String - `path` -> a String. Interestingly `settings`, `parserOptions` and `parserPath` rarely do change for us, so calculating their hashes on every iteration seems unnecessary. `hashObject` recursively calculates the hashes of each key / value pair. So instead of doing: ```js cacheKey = hashObject({settings, parserOptions, parserPath, path}) ``` We could also do: ```js cacheKey = parserPath + hashObject(parserOptions) + hashObject(settings) + path ``` This would be just as stable as before, although resulting in longer cache keys. `parserPath` and `path` would not need to be hashed, because they are strings and a single character change in them would result in a different cache key. Furthermore we can memoize the hashes of `parserOptions` and `settings`, in case they didn't change compared. The equality is checked with a simple `JSON.stringify`. We move this `cacheKey` calculation to `childContext`, adding the cache key to the `context` object. This way, we can fall back to the old calculation inside of `ExportMap.for`, as it is a public interface which consumers might be using. In our code base the results speak for itself: - 51.59s spent in `ExportMap.for`, 0ms spent in `childContext`. - 16.89s is spent in node:crypto/hash `update` (overall) - 41.02s spent in `ExportMap.for, 1.91s spent in `childContext`. - Almost no time spent in `hashObject`, actually all calls in our flame graph come from other code paths - 7.86s is spent in node:crypto/hash `update` (overall) So on this machine, and project, we are cutting the execution time of `ExportMap.for` in half. On machines, which are hashing slower, the effect might be more pronounced. Similarly machines with less memory, as the `hashObject` function creates a lot of tiny strings. One side-effect here could be, that the memoization is in-efficient if the `settings` or `parserOptions` change often. (I cannot think of such a scenario, but I am not that versed in the use cases of this plugin.) But even then, the overhead should mainly be the `JSON.stringify`.
- Loading branch information