diff --git a/.changeset/large-moons-nail.md b/.changeset/large-moons-nail.md new file mode 100644 index 0000000000..9e6390c0de --- /dev/null +++ b/.changeset/large-moons-nail.md @@ -0,0 +1,7 @@ +--- +"@siteimprove/alfa-cache": minor +--- + +**Added:** A `Cache.memoize` decorator is now available. + +It can decorate methods, or wrap functions, whose parameters are all objects. It will automatically create a cache with the various parameters and correctly call it. diff --git a/docs/review/api/alfa-cache.api.md b/docs/review/api/alfa-cache.api.md index 258319f149..5891e8e655 100644 --- a/docs/review/api/alfa-cache.api.md +++ b/docs/review/api/alfa-cache.api.md @@ -8,26 +8,22 @@ import { Iterable as Iterable_2 } from '@siteimprove/alfa-iterable'; import type { Mapper } from '@siteimprove/alfa-mapper'; import { Option } from '@siteimprove/alfa-option'; -// @public (undocumented) -export class Cache { - // (undocumented) - static empty(): Cache; - // (undocumented) +// @public +export class Cache { + static empty(): Cache; get(key: K): Option; - // (undocumented) get(key: K, ifMissing: Mapper): V; - // (undocumented) has(key: K): boolean; - // (undocumented) merge(iterable: Iterable_2): this; - // (undocumented) set(key: K, value: V): this; } // @public (undocumented) export namespace Cache { - // (undocumented) - export function from(iterable: Iterable_2): Cache; + export function from(iterable: Iterable_2): Cache; + export type Key = object; + export function memoize, Return>(target: (this: This, ...args: Args) => Return): (this: This, ...args: Args) => Return; + export function memoize, Return>(target: (...args: Args) => Return): (...args: Args) => Return; } // (No @packageDocumentation comment for this package) diff --git a/packages/alfa-aria/src/dom/predicate/is-programmatically-hidden.ts b/packages/alfa-aria/src/dom/predicate/is-programmatically-hidden.ts index d184d373dc..340106f7bc 100644 --- a/packages/alfa-aria/src/dom/predicate/is-programmatically-hidden.ts +++ b/packages/alfa-aria/src/dom/predicate/is-programmatically-hidden.ts @@ -7,7 +7,7 @@ import { Context } from "@siteimprove/alfa-selector"; import { Style } from "@siteimprove/alfa-style"; const { hasAttribute, isElement } = Element; -const { or, test, equals } = Predicate; +const { or, equals } = Predicate; const { and } = Refinement; const { hasComputedStyle } = Style; @@ -34,39 +34,26 @@ export function isProgrammaticallyHidden( ); } -const cache = Cache.empty>>(); - -function hasHiddenAncestors( +const hasHiddenAncestors = Cache.memoize(function ( device: Device, - context: Context = Context.empty(), + context: Context, ): Predicate { - return (node) => - cache - .get(device, Cache.empty) - .get(context, Cache.empty) - .get(node, () => - test( - or( - // Either it is a programmatically hidden element - and( - isElement, - or( - hasComputedStyle( - "display", - ({ values: [outside] }) => outside.value === "none", - device, - context, - ), - hasAttribute("aria-hidden", equals("true")), - ), - ), - // Or its parent is programmatically hidden - (node: Node) => - node - .parent(Node.fullTree) - .some(hasHiddenAncestors(device, context)), - ), - node, + return or( + // Either it is a programmatically hidden element + and( + isElement, + or( + hasComputedStyle( + "display", + ({ values: [outside] }) => outside.value === "none", + device, + context, ), - ); -} + hasAttribute("aria-hidden", equals("true")), + ), + ), + // Or its parent is programmatically hidden + (node: Node) => + node.parent(Node.fullTree).some(hasHiddenAncestors(device, context)), + ); +}); diff --git a/packages/alfa-cache/src/cache.ts b/packages/alfa-cache/src/cache.ts index 0f09037902..a0e875dac1 100644 --- a/packages/alfa-cache/src/cache.ts +++ b/packages/alfa-cache/src/cache.ts @@ -3,10 +3,32 @@ import { Option, None } from "@siteimprove/alfa-option"; import type { Mapper } from "@siteimprove/alfa-mapper"; /** + * Caches are wrapper around Javascript's `WeakMap` to store transient values. + * + * @remarks + * Caches are mutable! To preserve referential transparency, the preferred way + * of using caches is to store them as a local variable (never send them as + * parameters); and to use a single `cache.get(key, () => …)` call to retrieve + * values from it. Ideally, use `Cache.memoize()` to create a memoized function. + * + * Since Caches are built on WeakMap, the keys must be objects. + * + * Since Caches are built on WeakMap, they do not prevent the garbage collection + * of keys, and the associated value is then freed too. This avoids memory leaks, + * and ensure a lightweight caching mechanism for objects that stay in memory for + * some time. + * + * Typical use of Caches is to store indirect values related to a DOM tree (e.g., + * the associated ARIA tree, …) Once the audit is done and the DOM tree is + * discarded, the cache is automatically freed. + * * @public */ -export class Cache { - public static empty(): Cache { +export class Cache { + /** + * Creates an empty cache. + */ + public static empty(): Cache { return new Cache(); } @@ -14,8 +36,15 @@ export class Cache { private constructor() {} + /** + * Returns the value (if it exists) associated with the given key. + */ public get(key: K): Option; + /** + * Returns the value associated with the given key; if it does not exist, + * evaluates `ifMissing`, store the result in the cache and returns it. + */ public get(key: K, ifMissing: Mapper): V; public get( @@ -43,15 +72,27 @@ export class Cache { return value; } + /** + * Tests whether a given key exists in the cache. + */ public has(key: K): boolean { return this._storage.has(key); } + /** + * Adds a key-value pair to a cache. + * + * @remarks + * Avoid using this. Prefer using the `ifMissing` parameter of `get()` instead. + */ public set(key: K, value: V): this { this._storage.set(key, value); return this; } + /** + * Merges a cache with an iterable of key-value pairs. + */ public merge(iterable: Iterable): this { return Iterable.reduce( iterable, @@ -65,9 +106,117 @@ export class Cache { * @public */ export namespace Cache { - export function from( + /** + * Allowed keys in a Cache. + */ + export type Key = object; + + /** + * Creates a new cache from an iterable of key-value pairs. + */ + export function from( iterable: Iterable, ): Cache { return Cache.empty().merge(iterable); } + + /** + * Turns `<[A, B, C], T>` into `Cache>>`. + */ + type ToCache, T> = Args extends [ + infer Head extends Key, + ...infer Tail extends Array, + ] + ? Cache> + : T; + + /** + * Memoizes a method. + */ + export function memoize, Return>( + // When called on an instance's method `target`, `this` is the instance. + target: (this: This, ...args: Args) => Return, + ): (this: This, ...args: Args) => Return; + + /** + * Memoizes a function + * + * @remarks + * When memoizing a recursive function, care must be taken to also memoize the + * recursive calls. This is best done by wrapping an anonymous function that + * recurses on the memoized function: + * `const foo = Cache.memoize(function (x: A): B { … foo(x2) … }` + */ + export function memoize, Return>( + target: (...args: Args) => Return, + ): (...args: Args) => Return; + + export function memoize, Return>( + // When called on an instance's method `target`, `this` is the instance. + target: (this: This, ...args: Args) => Return, + ): (this: This, ...args: Args) => Return { + // First, we create the cache. + const cache = Cache.empty() as ToCache; + + // Next, we create the memoized function. Since the cache is scoped to the + // decorator, it cannot be accessed from outside and won't be tampered with. + return function (this: This, ...args: Args) { + // Here, `this` is still the instance on which the (new) method is added. + // We need to save it for later. + const that = this; + + // We create a recursive memoized function that will traverse the cache, + // parameter by parameter. It needs to be passed a (partial) cache + // together with the remaining parameters. + // This is OK since the side-effect happens only to the previously defined + // scoped cache. + function memoized>( + cache: ToCache, + ...innerArgs: A + ): Return { + // From now on, `this` is the `memoized` function itself, hence the need + // for an earlier copy. + + if (innerArgs.length === 0) { + // We have reached the end of the parameters, always hitting the cache, + // thus `ToCache` is `Return`, and `cache` is the actual + // return value that was `.get()` in the previous call. + + // Typescript is completely lost here. It cannot make the connection + // between `innerArgs` being of length 0, and `A` being `[]`; thus is + // unable to correctly infer that `ToCache` is `Return`. + return cache as Return; + } + + // There are still parameters to handle, deconstruct them. + const [head, ...tail] = innerArgs; + + // On that bit, TS is so lost that we just disable it… + // @ts-ignore + + // Compute the next cache to use, by retrieving the values associated + // with `head`. This will be either the final value (if `head` is the last + // parameter), or a further cache. + const next = cache.get( + head, + // @ts-ignore + // If `head` is not in the cache, and there are no more parameters, + // we need to call the original function. In case of method, we need + // to re-bind it to the original instance. + // (we could directly return the result in that case, instead of going + // to the next call to `memoized`; but since we need to test + // `innerArgs.length === 0` anyway, we let that handle it) + // + // If `head` is not in the cache but there are more parameters, + // we just create an empty cache. + tail.length === 0 ? () => target.bind(that)(...args) : Cache.empty, + ); + + // Recurse with the next cache and the remaining parameters. + return memoized(next, ...tail); + } + + return memoized(cache, ...args); + }; + } } diff --git a/packages/alfa-cache/test/cache.spec.ts b/packages/alfa-cache/test/cache.spec.ts index 868823c139..1da4968f54 100644 --- a/packages/alfa-cache/test/cache.spec.ts +++ b/packages/alfa-cache/test/cache.spec.ts @@ -3,19 +3,249 @@ import { test } from "@siteimprove/alfa-test"; import { None, Some } from "@siteimprove/alfa-option"; import { Cache } from "../dist/cache.js"; -const foo = {}; -const bar = {}; -const baz = {}; +type Foo = { x: number }; +type Bar = { y: string }; +const zero: Foo = { x: 0 }; +const one: Foo = { x: 1 }; +const two: Foo = { x: 2 }; + +const a: Bar = { y: "a" }; +const b: Bar = { y: "bbbbbbbbbb" }; + +// This effectively does a basic test of Cache.merge const cache = Cache.from([ - [foo, 1], - [bar, 2], + [zero, 0], + [one, 1], ]); -test("get() returns some when getting a value that does exist", (t) => { - t.deepEqual(cache.get(foo), Some.of(1)); +test("has()/get() returns some when getting a value that does exist", (t) => { + t(cache.has(zero)); + t.deepEqual(cache.get(zero), Some.of(0)); +}); + +test("has()/get() returns none when getting a value that does not exist", (t) => { + // We do not want to use `two` here to avoid race condition if tests are + // run asynchronously. + t(!cache.has({ x: 2 })); + t.equal(cache.get({ x: 2 }), None); +}); + +test("set() adds a value to a cache", (t) => { + cache.set(two, 2); + t(cache.has(two)); + t.deepEqual(cache.get(two), Some.of(2)); +}); + +test("get() adds a value to a cache when ifMissing is provided", (t) => { + const three: Foo = { x: 3 }; + const value = cache.get(three, () => 3); + + t(cache.has(three)); + t.equal(value, 3); +}); + +test("memoize caches values of a unary function", (t) => { + // We also test the return values of `doStuff` to ensure that we didn't retrieve + // the wrong cache entry. + let called = 0; + + function doStuff(foo: Foo): number { + called++; + return foo.x; + } + + // Not memoized, `called` is incremented each time + t.equal(doStuff(zero), 0); + t.equal(called, 1); + + t.equal(doStuff(zero), 0); + t.equal(called, 2); + + t.equal(doStuff(one), 1); + t.equal(called, 3); + + const memoized = Cache.memoize(doStuff); + + // Memoized, `called` is incremented only in case of cache miss. + t.equal(memoized(zero), 0); // Initial call, miss + t.equal(called, 4); + + t.equal(memoized(zero), 0); // hit + t.equal(called, 4); + + t.equal(memoized(one), 1); // different argument, miss + t.equal(called, 5); + + t.equal(memoized(one), 1); // hit + t.equal(called, 5); + + t.equal(memoized(zero), 0); // still a hit + t.equal(called, 5); +}); + +test("memoize caches values of a binary function", (t) => { + let called = 0; + + function doStuff(foo: Foo, bar: Bar): number { + called++; + + return foo.x + bar.y.length; + } + + // Not memoized, `called` is incremented each time + t.equal(doStuff(zero, a), 1); + t.equal(called, 1); + + t.equal(doStuff(one, a), 2); + t.equal(called, 2); + + t.equal(doStuff(zero, a), 1); + t.equal(called, 3); + + const memoize = Cache.memoize(doStuff); + + // Memoized, `called` is incremented only in case of cache miss. + t.equal(memoize(zero, a), 1); // Initial call, miss + t.equal(called, 4); + + t.equal(memoize(one, a), 2); // different foo, miss + t.equal(called, 5); + + t.equal(memoize(zero, a), 1); // hit (same as 1st) + t.equal(called, 5); + + t.equal(memoize(one, a), 2); // hit (same as 2nd) + t.equal(called, 5); + + t.equal(memoize(zero, b), 10); // different bar, miss + t.equal(called, 6); + + t.equal(memoize(zero, b), 10); // hit (same as 5th) + t.equal(called, 6); + + t.equal(memoize(one, b), 11); // different pair, miss + t.equal(called, 7); }); -test("get() returns none when getting a value that does not exist", (t) => { - t.equal(cache.get(baz), None); +test("@memoize caches values of a binary method", (t) => { + // Here also, we test the return values of `doStuffA` / `doStuffB` to ensure + // that we didn't retrieve the wrong cache entry. + + class MyClass { + public called: number; + + public constructor() { + this.called = 0; + } + + public doStuffA(foo: Foo, bar: Bar): number { + this.called++; + + return foo.x + bar.y.length; + } + + @Cache.memoize + public doStuffB(foo: Foo, bar: Bar): number { + this.called++; + + return foo.x + bar.y.length; + } + } + + const instance = new MyClass(); + + // doStuffA is not cached, `called` is incremented each time + t.equal(instance.doStuffA(zero, a), 1); + t.equal(instance.called, 1); + + t.equal(instance.doStuffA(one, a), 2); + t.equal(instance.called, 2); + + t.equal(instance.doStuffA(zero, a), 1); + t.equal(instance.called, 3); + + // doStuffB is cached, `called` is incremented only in case of cache miss + t.equal(instance.doStuffB(zero, a), 1); // Initial call, miss + t.equal(instance.called, 4); + + t.equal(instance.doStuffB(one, a), 2); // different foo, miss + t.equal(instance.called, 5); + + t.equal(instance.doStuffB(zero, a), 1); // hit (same as 1st) + t.equal(instance.called, 5); + + t.equal(instance.doStuffB(one, a), 2); // hit (same as 2nd) + t.equal(instance.called, 5); + + t.equal(instance.doStuffB(zero, b), 10); // different bar, miss + t.equal(instance.called, 6); + + t.equal(instance.doStuffB(zero, b), 10); // hit (same as 5th) + t.equal(instance.called, 6); + + t.equal(instance.doStuffB(one, b), 11); // different pair, miss + t.equal(instance.called, 7); +}); + +test("memoize() caches a recursive function when used correctly", (t) => { + type Foo = { x: number; y: Foo | undefined }; + const zero: Foo = { x: 0, y: undefined }; + const one: Foo = { x: 1, y: zero }; + const two: Foo = { x: 2, y: one }; + const three: Foo = { x: 3, y: two }; + + let called = 0; + + const memoized = Cache.memoize(function (foo: Foo): number { + called++; + + return foo.x + (foo.y ? memoized(foo.y) : 0); + }); + + t.equal(called, 0); + + // Initial call, fills the cache for all values it recurses on + t.equal(memoized(three), 6); + t.equal(called, 4); + + // Same call, gets the result immediately + t.equal(memoized(three), 6); + t.equal(called, 4); + + // Call on a sub-object, the cache was still correctly filled. + t.equal(memoized(two), 3); + t.equal(called, 4); +}); + +test("memoize() does not fully cache a recursive function when used incorrectly", (t) => { + type Foo = { x: number; y: Foo | undefined }; + const zero: Foo = { x: 0, y: undefined }; + const one: Foo = { x: 1, y: zero }; + const two: Foo = { x: 2, y: one }; + const three: Foo = { x: 3, y: two }; + + let called = 0; + + function sum(foo: Foo): number { + called++; + + return foo.x + (foo.y ? sum(foo.y) : 0); + } + const memoized = Cache.memoize(sum); + + t.equal(called, 0); + + // Initial call, fills the cache for all values it recurses on + t.equal(memoized(three), 6); + t.equal(called, 4); + + // Same call, gets the result immediately + t.equal(memoized(three), 6); + t.equal(called, 4); + + // Call on a sub-object, the cache was not filled since the recursion was + // done on the original function. + t.equal(memoized(two), 3); + t.equal(called, 7); });