Skip to content

Turn math cache into its own object#897

Open
xmariachi wants to merge 4 commits into
devfrom
diego/engn-4936-refactor-math-cache-from-being-global-var-into-struct-based
Open

Turn math cache into its own object#897
xmariachi wants to merge 4 commits into
devfrom
diego/engn-4936-refactor-math-cache-from-being-global-var-into-struct-based

Conversation

@xmariachi

@xmariachi xmariachi commented Dec 23, 2025

Copy link
Copy Markdown
Contributor

Purpose of Changes and their Description

Turning math cache into an object instead of using global vars.

Are these changes tested and documented?

  • If tested, please describe how. If not, why tests are not needed. -- Added extensive unit tests, in both the cache and the usage.
  • If documented, please describe where. If not, describe why docs are not needed. -- It is a performance change, not a functionality change, so it should not need its own documentation.
  • Added to Unreleased section of CHANGELOG.md?

@spooktheducks spooktheducks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Per comments in private messages with @jmdkastro and @xmariachi , I say we hold on approvals here pending:

  • solid benchmarking showing that this cache is actually necessary (as opposed to just causing code bloat)
  • a more performantly constructed key for the cache map (we can reduce Decimal into hashable bytes much more efficiently than converting to string with fmt.Sprintf)
  • an understanding as to why we're fragmenting the cache, which somewhat negates the effects of caching math in the first place

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants