feat(#5165): add PhSticky memoizing decorator

[asmirnov-backend]

Part of #5165. First vertical slice toward memoization: the decorator and its transpilation seam.

PhSticky memoizes the result of take for a single targeted expression, keyed on a compile-time constant locator baked in by the transpiler (key.name). Deciding whether to memoize is one string comparison — it never activates, dataizes nor reflects upon the wrapped object (unlike φTerm(), which would). The first real target is Φ.nan.is-finite; a second taking of it anywhere yields the very same Phi. Every other expression is delegated and rebuilt as usual.

to-java.xsl wraps every formed object in new PhSticky(origin, "<loc>"), baking the object's @loc as the key and putting the seam in place.

Follow-ups in #5165: value-level keying via φTerm() (the baked locator is source identity, enough for context-free constants like nan) and purity gating via the Impure annotation (#5245), so impure objects are never shared.

[github-actions]

🚀 Performance Analysis

All benchmarks are within the acceptable range. No critical degradation detected (threshold is 100%). Please refer to the detailed report for more information.

Click to see the detailed report

Test	Base Score	PR Score	Change	% Change	Unit	Mode
benchmarks.XmirBench.xmirToEO	11092.178	10891.549	-200.629	-1.81%	ms/op	Average Time

✅ Performance gain: benchmarks.XmirBench.xmirToEO is faster by 200.629 ms/op (1.81%)

[yegor256]

@asmirnov-backend this is a good piece of code, but it's still disconnected from the rest of the code base. Imagine, we merge this and tomorrow someone will open this repo and ask a question: "What this class is doing here, what it's for?" He won't find an answer to this question and will delete the class. Read again: https://www.yegor256.com/2015/02/12/top-down-design.html

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[asmirnov-backend]

@yegor256 could you please take a look again. Now PhSticky memo only Φ.nan.is-finite

[gemshrine]

Three concerns about the invariants held by this decorator — see the inline comments on equals, take, and copy.

[gemshrine]

PhSticky.equals(x) returns origin.equals(x); that reads as symmetric relative to origin, but relative to this it isn't. For any x where origin.equals(x) == true:

• phSticky.equals(x) → origin.equals(x) → true
• x.equals(phSticky) → typically obj instanceof <x's class> (or an identity check) → false, because phSticky is a PhSticky, not the class x recognises as equal to itself.

That violates the symmetry clause of Object.equals and produces inconsistent HashMap / Set / List.contains behaviour depending on which side of the pair is the receiver.

The same delegation also breaks reflexivity in the common case: phSticky.equals(phSticky) → origin.equals(phSticky) → usually false, because origin doesn't unwrap PhSticky and doesn't hold phSticky by identity.

A symmetric shape:

@Override
public boolean equals(final Object obj) {
    final Phi other;
    if (obj instanceof PhSticky) {
        other = ((PhSticky) obj).origin;
    } else {
        other = (Phi) obj;
    }
    return this.origin.equals(other);
}

[gemshrine]

MEMORY is keyed only by the constant KEY, not by origin identity. Because to-java.xsl wraps every formed object as new PhSticky(origin, "@loc"), any subtree whose composite locator + "." + name equals "Φ.nan.is-finite" — regardless of what origin is — hits this same slot and gets whatever Phi the first caller inserted:

Phi first  = new PhSticky(Phi.Φ.take("nan"), "Φ.nan").take("is-finite");
Phi second = new PhSticky(mockOrigin,        "Φ.nan").take("is-finite");
// second == first  (mockOrigin.take never runs, mockOrigin's answer is lost)

For the currently-scoped constant Φ.nan.is-finite this is probably intended (there's a single "true" origin at that locator in the whole codebase). But the invariant on which correctness rests — that the transpiler-baked locator uniquely identifies the origin behaviour — is a hidden coupling with the rest of the pipeline. Worth documenting on the class so a future addition to KEY doesn't silently violate it.

A defensive tightening: also key on origin.forma() (or System.identityHashCode(this.origin) if forma() is too heavy on the hot path), so a mismatched wrapping can't poison the cache for the real target.

[gemshrine]

copy() returns a fresh PhSticky wrapping origin.copy(), but MEMORY is process-wide keyed only on KEY. Once the original has populated MEMORY[KEY], calling take("is-finite") on the copy silently returns the original's cached Phi, not origin.copy().take("is-finite"). Any client that uses copy() to obtain an independent object — expecting its taken attributes to be built afresh so mutations to the copy don't leak into the original — gets a shared instance here for the targeted expression.

If that's fine for the class of constants covered by this initial slice (the class javadoc argues Φ.nan is context-free), it'd help to spell out on the class that copy and sticky interact this way, because at the Phi API level a .copy() whose taken attribute is sameInstance as the original's is surprising.

[morphqdd]

Read the actual PR diff and the wrapping logic in to-java.xsl closely. The narrow scope (one hardcoded target, nan.is-finite) is a reasonable first vertical slice and I don't see a correctness bug in the memoization itself. But the implementation raises real design and performance questions worth resolving before this pattern gets extended to more targets, since a few of these get harder to fix once other PRs build on top of this shape.

[morphqdd]

This unconditionally wraps every formed object in the entire generated codebase in new PhSticky(...), to benefit exactly one hardcoded expression (nan.is-finite). That is a real allocation cost paid everywhere for a benefit that applies nowhere except one call site. Since the transpiler already knows @loc statically at this point, could the wrap itself be gated here (e.g. <xsl:if test="@loc = 'Φ.nan'">) instead of wrapping unconditionally and pushing the filtering into PhSticky.take() at runtime?

[morphqdd]

KEY is a single hardcoded string and MEMORY is effectively a one-entry map (new ConcurrentHashMap<>(1)). The PR description frames this as a "first vertical slice toward memoization" with more targets to follow, but as written this class has no path to a second target without being rewritten: there's no way to add a key without touching this exact string comparison. If the intent is general memoization, would it be worth structuring this now as a set of keys (even a single-element one to start) so the next increment is additive instead of a rewrite?

[morphqdd]

String.join(".", this.locator, name) allocates a new string on every take() call, for every wrapped object in the program, just to compare against one constant. Given every formed object is now wrapped (see the to-java.xsl comment), this runs constantly. Checking "is-finite".equals(name) first would let almost every call short-circuit before any allocation, since the vast majority of take() calls will never be for is-finite at all.

[morphqdd]

computeIfAbsent with a mapping function that calls back into this.origin.take(name) is safe here because nan.is-finite is a leaf computation with no further sticky-hitting dependencies. But this is exactly the shape (ConcurrentHashMap.computeIfAbsent whose lambda can re-enter the same map) that throws IllegalStateException: Recursive update in the JDK if the computation ever indirectly calls back into a take() for the same key. Worth a comment here, or a note in the class javadoc, warning future sticky targets away from anything that could recurse through the same key, since nothing here would catch that until it hangs or throws in production.

[morphqdd]

MEMORY is static, process-wide, and never evicted. Harmless when it holds exactly one entry forever, but worth deciding now, before more targets are added, whether this needs an eviction policy or a bound. Retrofitting that later, once multiple call sites depend on the current unbounded behavior, is more work than deciding it now.

[morphqdd]

This test populates the real, static, process-wide nan.is-finite cache entry for the remainder of the JVM's life. eo-runtime's test suite runs with junit.jupiter.execution.parallel.enabled = true (see the module's pom.xml). Is there anything elsewhere in the suite that dataizes nan.is-finite and could be affected by whether this test has already run and populated the cache? This is the same class of bug (claimed-safe shared static state under parallel test execution) that turned out to be real in ConcurrentCache (issue #5720) elsewhere in this codebase. Worth confirming there's no order dependency, or resetting MEMORY between tests if that's feasible.

[morphqdd]

None of the three tests exercise actual concurrent access to MEMORY, only sequential calls. The whole reason it's a ConcurrentHashMap is presumably to be safe under concurrent take() calls from multiple threads, but that property is asserted by the type choice, not verified by a test. A test that hits take() from multiple threads concurrently and asserts every thread observes the same instance would actually verify the thread-safety claim.

[morphqdd]

"but it didnt" is missing an apostrophe, should be "didn't". Same typo repeated in the other two test messages below (lines 32 and 45).

[morphqdd]

This branch is reported BEHIND master and the raw diff against current upstream/master shows this is missing several recent to-java.xsl changes (the trackLocations/coverage params, PhTerminator handling, eo:package-name helpers). Worth rebasing before this lands, so the two new sticky call sites are actually tested against the current shape of the file rather than an older one.

[morphqdd]

Two more findings after digging further, on top of the review I already submitted.

[morphqdd]

This branch's own Phi.java ends after forma(), no normalized() method. Current upstream/master's Phi.java has a normalized() method with no default implementation (Phi normalized();), added after this branch diverged. PhSticky does not implement it. This compiles fine on this branch today, but once this is rebased onto current master, PhSticky implements Phi will fail to compile until normalized() is added here too. Worth handling as part of the rebase already flagged in the other review, not after, since it will block the build otherwise.

[morphqdd]

//java[contains(text(), 'rb1 = new PhSticky(rb1, "')] only checks that a PhSticky wrapper appears, not that the @loc value baked into it is correct. Since the entire correctness of the memoization depends on the baked locator being exactly right (a wrong or inconsistent locator would either silently disable memoization by never matching KEY, or worse, match across two genuinely different expressions), this test would not catch either failure mode. Worth asserting the full generated line, or at least the specific locator value, not just that some PhSticky(...) call is present.