From a6d8f73f29be6d256cd0a7d167b3ca5336c47c57 Mon Sep 17 00:00:00 2001 From: mcpower Date: Sat, 26 Mar 2022 13:40:58 +1100 Subject: [PATCH 1/6] Add simple LRU cache --- src/lru-cache.ts | 134 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 src/lru-cache.ts diff --git a/src/lru-cache.ts b/src/lru-cache.ts new file mode 100644 index 0000000..e6ee765 --- /dev/null +++ b/src/lru-cache.ts @@ -0,0 +1,134 @@ +/** + * A LRU cache intended for caching pure functions. + */ +export class LRUCache { + private map = new Map>(); + // Invariant: Exactly one of the below is true before and after calling a + // LRUCache method: + // - first and last are both undefined, and map.size() is 0. + // - first and last are the same object, and map.size() is 1. + // - first and last are different objects, and map.size() is greater than 1. + private first: ListNode | undefined = undefined; + private last: ListNode | undefined = undefined; + maxSize: number; + + /** + * @param maxSize The maximum size for this cache. + */ + constructor(maxSize: number) { + this.maxSize = maxSize; + } + + get size(): number { + return this.map.size; + } + + /** + * Gets the specified key from the cache, or undefined if it is not in the + * cache. + * @param key The key to get. + * @returns The cached value, or undefined if key is not in the cache. + */ + get(key: K): V | undefined { + const node = this.map.get(key); + if (node === undefined) { + return undefined; + } + // It is guaranteed that there is at least one item in the cache. + // Therefore, first and last are guaranteed to be a ListNode... + // but if there is only one item, they might be the same. + + // Update the order of the list to make this node the first node in the + // list. + // This isn't needed if this node is already the first node in the list. + if (node !== this.first) { + // As this node is DIFFERENT from the first node, it is guaranteed that + // there are at least two items in the cache. + // However, this node could possibly be the last item. + if (node === this.last) { + // This node IS the last node. + this.last = node.prev; + // From the invariants, there must be at least two items in the cache, + // so node - which is the original "last node" - must have a defined + // previous node. Therefore, this.last - set above - must be defined + // here. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this.last!.next = undefined; + } else { + // This node is somewhere in the middle of the list, so there must be at + // least THREE items in the list, and this node's prev and next must be + // defined here. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + node.prev!.next = node.next; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + node.next!.prev = node.prev; + } + node.next = this.first; + // From the invariants, there must be at least two items in the cache, so + // this.first must be a valid ListNode. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this.first!.prev = node; + this.first = node; + } + return node.value; + } + + /** + * Sets an entry in the cache. + * + * @param key The key of the entry. + * @param value The value of the entry. + * @throws Error, if the map already contains the key. + */ + set(key: K, value: V): void { + // Ensure that this.maxSize >= 1. + if (this.maxSize < 1) { + return; + } + if (this.map.has(key)) { + throw new Error("Cannot update existing keys in the cache"); + } + const node = new ListNode(key, value); + // Move node to the front of the list. + if (this.first === undefined) { + // If the first is undefined, the last is undefined too. + // Therefore, this cache has no items in it. + this.first = node; + this.last = node; + } else { + // This cache has at least one item in it. + node.next = this.first; + this.first.prev = node; + this.first = node; + } + this.map.set(key, node); + + while (this.map.size > this.maxSize) { + // We are guaranteed that this.maxSize >= 1, + // so this.map.size is guaranteed to be >= 2, + // so this.first and this.last must be different valid ListNodes, + // and this.last.prev must also be a valid ListNode (possibly this.first). + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const last = this.last!; + this.map.delete(last.key); + this.last = last.prev; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this.last!.next = undefined; + } + } +} + +/** + * A node in a doubly linked list. + */ +class ListNode { + key: K; + value: V; + next: ListNode | undefined = undefined; + prev: ListNode | undefined = undefined; + + constructor(key: K, value: V) { + this.key = key; + this.value = value; + } +} From dc5e216136fdc6bd3ad4a3882d62e156808ae6a5 Mon Sep 17 00:00:00 2001 From: mcpower Date: Sat, 26 Mar 2022 14:01:43 +1100 Subject: [PATCH 2/6] Add cache for fromString In Prestige Tree Rewritten, Decimal.fromString takes 16% of scripting time, mostly called from the constructor (which is called from D). Strings constants are passed into Decimal methods, which get converted into new Decimals using D. These string constants are being converted into strings every time those methods are called. To fix this, Prestige Tree Rewritten could instead pre-convert all string constants used to be Decimals, then use those Decimals instead. However, that would require a LOT of new code and would be unwieldy to use. Instead, we can implement caching for fromString from break_eternity.js's end. This reduces the aforementioned 16% of scripting time down to 0.6% without changing a line of game code. --- src/index.ts | 55 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/src/index.ts b/src/index.ts index 60e78b2..8799e0d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,3 +1,5 @@ +import { LRUCache } from "./lru-cache"; + export type CompareResult = -1 | 0 | 1; const MAX_SIGNIFICANT_DIGITS = 17; //Maximum number of digits of precision to assume in Number @@ -14,6 +16,8 @@ const NUMBER_EXP_MIN = -324; //The smallest exponent that can appear in a Number const MAX_ES_IN_A_ROW = 5; //For default toString behaviour, when to swap from eee... to (e^n) syntax. +const DEFAULT_FROM_STRING_CACHE_SIZE = 1 << 10; // The default size of the LRU cache used to cache Decimal.fromString. + const IGNORE_COMMAS = true; const COMMAS_ARE_DECIMAL_POINTS = false; @@ -348,6 +352,8 @@ export default class Decimal { public static readonly dNumberMax = FC(1, 0, Number.MAX_VALUE); public static readonly dNumberMin = FC(1, 0, Number.MIN_VALUE); + private static fromStringCache = new LRUCache(DEFAULT_FROM_STRING_CACHE_SIZE); + public sign: number = Number.NaN; public mag: number = Number.NaN; public layer: number = Number.NaN; @@ -1156,6 +1162,11 @@ export default class Decimal { } public fromString(value: string): Decimal { + const originalValue = value; + const cached = Decimal.fromStringCache.get(originalValue); + if (cached !== undefined) { + return this.fromDecimal(cached); + } if (IGNORE_COMMAS) { value = value.replace(",", ""); } else if (COMMAS_ARE_DECIMAL_POINTS) { @@ -1180,6 +1191,9 @@ export default class Decimal { this.sign = result.sign; this.layer = result.layer; this.mag = result.mag; + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } } @@ -1202,6 +1216,9 @@ export default class Decimal { this.sign = result.sign; this.layer = result.layer; this.mag = result.mag; + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } } @@ -1216,6 +1233,9 @@ export default class Decimal { this.sign = result.sign; this.layer = result.layer; this.mag = result.mag; + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } } @@ -1241,6 +1261,9 @@ export default class Decimal { this.sign = result.sign; this.layer = result.layer; this.mag = result.mag; + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } } @@ -1261,6 +1284,9 @@ export default class Decimal { this.sign = result.sign; this.layer = result.layer; this.mag = result.mag; + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } } @@ -1272,13 +1298,21 @@ export default class Decimal { if (ecount === 0) { const numberAttempt = parseFloat(value); if (isFinite(numberAttempt)) { - return this.fromNumber(numberAttempt); + this.fromNumber(numberAttempt); + if (Decimal.fromStringCache.size >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } + return this; } } else if (ecount === 1) { //Very small numbers ("2e-3000" and so on) may look like valid floats but round to 0. const numberAttempt = parseFloat(value); if (isFinite(numberAttempt) && numberAttempt !== 0) { - return this.fromNumber(numberAttempt); + this.fromNumber(numberAttempt); + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } + return this; } } @@ -1300,6 +1334,9 @@ export default class Decimal { this.layer = parseFloat(layerstring); this.mag = parseFloat(newparts[1].substr(i + 1)); this.normalize(); + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } } @@ -1309,6 +1346,9 @@ export default class Decimal { this.sign = 0; this.layer = 0; this.mag = 0; + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } const mantissa = parseFloat(parts[0]); @@ -1316,6 +1356,9 @@ export default class Decimal { this.sign = 0; this.layer = 0; this.mag = 0; + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } let exponent = parseFloat(parts[parts.length - 1]); @@ -1350,6 +1393,9 @@ export default class Decimal { this.sign = result.sign; this.layer = result.layer; this.mag = result.mag; + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } else { //at eee and above, mantissa is too small to be recognizable! @@ -1358,6 +1404,9 @@ export default class Decimal { } this.normalize(); + if (Decimal.fromStringCache.maxSize >= 1) { + Decimal.fromStringCache.set(originalValue, Decimal.fromDecimal(this)); + } return this; } @@ -2067,7 +2116,7 @@ export default class Decimal { } else if (Math.abs(b.toNumber() % 2) % 2 === 0) { return result; } - return Decimal.dNaN; + return result; } return result; From 38bcc55a0241315f635656e9eec66053eb91acf1 Mon Sep 17 00:00:00 2001 From: mcpower Date: Sat, 26 Mar 2022 14:06:48 +1100 Subject: [PATCH 3/6] Return cached Decimals directly from D This commit also in-lines the type checks from the constructor into D, reducing the need for un-necessary type-checks in the constructor. --- src/index.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 8799e0d..98bfb2a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -495,7 +495,22 @@ export default class Decimal { * is required. */ public static fromValue_noAlloc(value: DecimalSource): Readonly { - return value instanceof Decimal ? value : new Decimal(value); + if (value instanceof Decimal) { + return value; + } else if (typeof value === "string") { + const cached = Decimal.fromStringCache.get(value); + if (cached !== undefined) { + return cached; + } + return Decimal.fromString(value); + } else if (typeof value === "number") { + return Decimal.fromNumber(value); + } else { + // This should never happen... but some users like Prestige Tree Rewritten + // pass undefined values in as DecimalSources, so we should handle this + // case to not break them. + return Decimal.dZero; + } } public static abs(value: DecimalSource): Decimal { From 51f7188ee2e681f69ec2f4acb35a0b2fdacaad9b Mon Sep 17 00:00:00 2001 From: mcpower Date: Sun, 27 Mar 2022 11:14:03 +1100 Subject: [PATCH 4/6] Subtract one from default fromString cache size See the newly added comment in lru-cache.ts for more information. --- src/index.ts | 2 +- src/lru-cache.ts | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 98bfb2a..8c3cbeb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,7 +16,7 @@ const NUMBER_EXP_MIN = -324; //The smallest exponent that can appear in a Number const MAX_ES_IN_A_ROW = 5; //For default toString behaviour, when to swap from eee... to (e^n) syntax. -const DEFAULT_FROM_STRING_CACHE_SIZE = 1 << 10; // The default size of the LRU cache used to cache Decimal.fromString. +const DEFAULT_FROM_STRING_CACHE_SIZE = 1 << (10 - 1); // The default size of the LRU cache used to cache Decimal.fromString. const IGNORE_COMMAS = true; const COMMAS_ARE_DECIMAL_POINTS = false; diff --git a/src/lru-cache.ts b/src/lru-cache.ts index e6ee765..ec78e6a 100644 --- a/src/lru-cache.ts +++ b/src/lru-cache.ts @@ -13,7 +13,12 @@ export class LRUCache { maxSize: number; /** - * @param maxSize The maximum size for this cache. + * @param maxSize The maximum size for this cache. We recommend setting this + * to be one less than a power of 2, as most hashtables - including V8's + * Object hashtable (https://crsrc.org/c/v8/src/objects/ordered-hash-table.cc) + * - uses powers of two for hashtable sizes. It can't exactly be a power of + * two, as a .set() call could temporarily set the size of the map to be + * maxSize + 1. */ constructor(maxSize: number) { this.maxSize = maxSize; From 41124f83dbbbd0c2bfd6e127e89d2a3564377452 Mon Sep 17 00:00:00 2001 From: mcpower Date: Sun, 27 Mar 2022 11:18:25 +1100 Subject: [PATCH 5/6] Fix order of operations for fromString cache size Prettier explicitly parenthesised `1 << 10 - 1` to `1 << (10 - 1)`, as bitwise operator precedence is pretty confusing. Thanks Prettier! --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 8c3cbeb..6d032df 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,7 +16,7 @@ const NUMBER_EXP_MIN = -324; //The smallest exponent that can appear in a Number const MAX_ES_IN_A_ROW = 5; //For default toString behaviour, when to swap from eee... to (e^n) syntax. -const DEFAULT_FROM_STRING_CACHE_SIZE = 1 << (10 - 1); // The default size of the LRU cache used to cache Decimal.fromString. +const DEFAULT_FROM_STRING_CACHE_SIZE = (1 << 10) - 1; // The default size of the LRU cache used to cache Decimal.fromString. const IGNORE_COMMAS = true; const COMMAS_ARE_DECIMAL_POINTS = false; From d99422cc2bed0d9b56af1cade9f0200616cc9ed9 Mon Sep 17 00:00:00 2001 From: mcpower Date: Tue, 28 Jun 2022 19:22:23 +1000 Subject: [PATCH 6/6] Revert testing code in Decimal.pow dc5e216136fdc6bd3ad4a3882d62e156808ae6a5 introduced an unintended functional change to Decimal.pow intended to be used for testing only. --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index e7cd428..fd988a9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2127,7 +2127,7 @@ export default class Decimal { } else if (Math.abs(b.toNumber() % 2) % 2 === 0) { return result; } - return result; + return Decimal.dNaN; } return result;