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

Add improved webpack cache #1916

Merged
merged 1 commit into from Jan 26, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 43 additions & 3 deletions packages/loader/lib/index.js
Expand Up @@ -5,13 +5,20 @@
* @typedef {Pick<CompileOptions, 'SourceMapGenerator'>} Defaults
* @typedef {Omit<CompileOptions, 'SourceMapGenerator'>} Options
* @typedef {import('webpack').LoaderContext<unknown>} LoaderContext
* @typedef {import('webpack').Compiler} WebpackCompiler
* @typedef {(vfileCompatible: VFileCompatible) => Promise<VFile>} Process
*/

import {createHash} from 'node:crypto'
import {SourceMapGenerator} from 'source-map'
import {createFormatAwareProcessors} from '@mdx-js/mdx/lib/util/create-format-aware-processors.js'

/** @type {WeakMap<CompileOptions, Process>} */
const own = {}.hasOwnProperty

// Note: the cache is heavily inspired by:
// <https://github.com/TypeStrong/ts-loader/blob/5c030bf/src/instance-cache.ts>
const marker = /** @type {WebpackCompiler} */ ({})
/** @type {WeakMap<WebpackCompiler, Map<string, Process>>} */
const cache = new WeakMap()

/**
Expand All @@ -28,6 +35,10 @@ export function loader(value, callback) {
const defaults = this.sourceMap ? {SourceMapGenerator} : {}
const options = /** @type {CompileOptions} */ (this.getOptions())
const config = {...defaults, ...options}
const hash = getOptionsHash(options)
// Some loaders set `undefined` (see `TypeStrong/ts-loader`).
/* c8 ignore next */
const compiler = this._compiler || marker

/* Removed option. */
/* c8 ignore next 5 */
Expand All @@ -37,15 +48,44 @@ export function loader(value, callback) {
)
}

let process = cache.get(config)
let map = cache.get(compiler)

if (!map) {
map = new Map()
cache.set(compiler, map)
}

let process = map.get(hash)

if (!process) {
process = createFormatAwareProcessors(config).process
cache.set(config, process)
map.set(hash, process)
}

process({value, path: this.resourcePath}).then((file) => {
callback(null, file.value, file.map)
return file
}, callback)
}

/**
* @param {Options} options
*/
function getOptionsHash(options) {
const hash = createHash('sha256')
Copy link
Member

Choose a reason for hiding this comment

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

Here speed would be of value, it may be worth considering a blake2 hash

Suggested change
const hash = createHash('sha256')
const hash = createHash('blake2s256')

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m a bit uneasy with a digest format that is not frequently used.
Searching Google or markdown on GitHub seems to almost exclusively include it in lists of all possible algorithms, not in code like us here or in tutorials or so.

I was also uneasy about a bundled/external dependency. It does seem like blake2s256 is supported in all maintained versions of Node.js though, as it seems to have landed in OpenSSL 1.1.1. So it seems okay, but I know that there are smaller Node.js builds out there as well?

I’m also assuming that the time factor is fine when using sha256, options objects should be rather small (especially as functions omitted) and performance bottlenecks of webpack and MDX are in other areas.

I do see room for improvement around hashing the options. In the area of preventing crashes on cyclical references in options, adding support for functions, and using a package to hash an options object which likely already exists,

Given that you’re 👍 on this PR without this change, I’ll merge it as is, but I’m open to discussing improvements to this PRs functionality.

Copy link
Member

Choose a reason for hiding this comment

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

the 2b variant is also an option, and is more common https://github.com/search?q=%22blake2b%22&type=Code
The reason I suggested 2s is it matches the hash size, and it optimized for 16/32 bit processors.
While 2b has a larger hash size and is optimized for 64 bit.

/** @type {keyof Options} */
let key

for (key in options) {
if (own.call(options, key)) {
const value = options[key]

if (value !== undefined) {
const valueString = JSON.stringify(value)
hash.update(key + valueString)
}
}
}

return hash.digest('hex').slice(0, 16)
}