Skip to content

Commit

Permalink
memoize monster-ability-part->rich-text
Browse files Browse the repository at this point in the history
The memoize procedure is not suited for anything outside its current
use, including for larger chunks of ability cards. (Knowing that
ability-text will always be a string avoids expensive model-equal?
checks, for example.)

Rationale
---------
Memoization in this case trades more baseline memory use for what I hope
will be increased performance via less garbage generation: knowing that
the app is slowest during the round (when abilities are displayed as
picts), I wonder if constantly recomputing the picts was a source of
issue. In theory, they are recomputed each time any part of the
creatures tree changes despite not usually being different. This might
create a lot of garbage to deal with? Instead, cache them all as they
are seen in a multi-layered cache. The layers avoid wrapping the
arguments and cache keys in (garbage) lists, although computing new
layers on-demand does create some small garbage lists.

Semantics
---------
Important: each hash-ref! result must be a thunk; otherwise, the
computation (f …) will be run before checking the cache, which defeats
the purpose.

Keys
----
It remains to be seen if a better mg-key can be chosen (for example,
"bonuses" can be omitted) without the overhead of computing the key
outweighing its size or ability to reduce duplication.

Since we are memoizing a function over parts, common parts like "Attack
+1" will share the next layer, which I find pleasing. Since env does not
change often, placing that next may improve sharing.

Extra syntax
------------
- make-hash over (list (cons ...) ...) is long, but there's no hash-like
  constructor for mutable hashes. Without pulling in syntax-parse I
  can't parse spliced {~@ k v} ... pairs, so group them with a minimum
  of notation to get close to (hash k v ...) for a mutable hash. It's
  local: don't try to make an immutable hash inside memoize, but that
  procedure should change rarely.
- Computing the key for the "mg" (monster group) layer is verbose and
  has to be repeated in a few places; use a short macro to help focus on
  the structural similarities of each line. (This was even more true in
  an uncommitted version of this code prior to df01908
  (monster-ability-ability->rich-text: remove unused argument,
  2024-11-30) when I was using the ability-card as a key for a layer of
  hashing: noticing that this would prevent much re-use of parts of the
  cache for identical ability cards is what prompted that commit.)
  • Loading branch information
benknoble committed Nov 30, 2024
1 parent 9ae44d2 commit f109c4e
Showing 1 changed file with 17 additions and 1 deletion.
18 changes: 17 additions & 1 deletion defns/monsters.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@
[(cons x xs) (list* "" x xs)]
[_ ability-parts]))

(define (monster-ability-part->rich-text ability-text mg env)
(define (monster-ability-part->rich-text* ability-text mg env)
(define attack
(list #px"(.*)((?i:attack))\\s+([+-])(\\d+)"
(skip-if-grant-or-control (keyword-sub {(monster-stats-attack* env)} mg))))
Expand Down Expand Up @@ -280,6 +280,22 @@
([pict-replacement (in-list pict-replacements)])
(append-map (only-on-text pict-replacement) result)))

;; highly specific to the monster-ability-part->rich-text!
(define (memoize f)
(define cache (make-hash))
(define-syntax-rule (hash [k v] ...)
(make-hash (list (cons k v) ...)))
(define-syntax-rule (mg-key mg)
(list (monster-group-normal-stats mg)
(monster-group-elite-stats mg)))
(λ (ability-text mg env)
(~> (cache)
(hash-ref! ability-text (thunk (hash [(mg-key mg) (hash [env (f ability-text mg env)])])))
(hash-ref! (mg-key mg) (thunk (hash [env (f ability-text mg env)])))
(hash-ref! env (thunk (f ability-text mg env))))))

(define monster-ability-part->rich-text (memoize monster-ability-part->rich-text*))

(module+ test
(require rackunit)
(define env (hash))
Expand Down

0 comments on commit f109c4e

Please sign in to comment.