test: pin identity-function optimization behavior (#815)#888
Open
He-Pin wants to merge 1 commit into
Open
Conversation
Motivation: sjsonnet has several aggressive identity-function fast paths: apply1 elides calls to (effectively) identity functions, StaticOptimizer statically classifies self-composition chains (staticIdentityShape), Val.Func caches an effective-identity state, and set/sort/uniq short-circuit keyF=id/default. These were untested as a group, so a future change could silently break an elision (e.g. start treating a non-identity lookalike as identity, or drop laziness). Issue databricks#815 asked for an audit + regression coverage. Modification: - Audit: verified every identity fast path against official Jsonnet v0.22.0 — all correct, including the safety-critical cases (non-identity lookalikes are NOT elided; laziness/traces preserved). - Add identity_function_optimization.jsonnet (new_test_suite) pinning: direct identity elision, self-composition over an identity g (depth 2 and 3, plus cached repeat calls), rejection of lookalikes (g(g(x)) with g=x+1; captured-var body; two-param function(x,y) x), keyF=id in sort/uniq/set/setUnion/setInter/setDiff, the function(x) -x sort fast path, and preserved laziness (identity map over an array containing `error` does not force it). - Add EvaluatorTests."identityFunctionTraces": the elision forces the argument exactly once (direct and self-composition) and a lazy identity map does not force unused elements (no spurious trace). Result: No behavior change — the audit found the optimizations sound; these tests lock in their semantics. Full JVM test suite green (EvaluatorTests runs both evaluator variants). References: databricks#815
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
sjsonnet has several aggressive identity-function fast paths:
apply1elides calls to (effectively) identity functions (Val.Func.isEffectivelyIdentity);StaticOptimizerstatically classifies self-composition chains (staticIdentityShape);Val.Funccaches an effective-identity state;set/sort/uniq/setUnion/setInter/setDiffshort-circuitkeyF=id/default, plus afunction(x) -xfast path.These were not tested as a group, so a future change could silently break an elision — e.g. start treating a non-identity lookalike as identity, or drop laziness. Issue #815 asked for an audit + regression coverage.
Modification
identity_function_optimization.jsonnet(new_test_suite) pins: direct identity elision; self-composition over an identityg(depth 2 and 3, plus cached repeat calls); rejection of lookalikes (g(g(x))withg=x+1; captured-var body; two-paramfunction(x, y) x);keyF=idin sort/uniq/set/setUnion/setInter/setDiff; thefunction(x) -xsort fast path; and preserved laziness (identitymapover an array containingerrordoes not force it).EvaluatorTests."identityFunctionTraces": the elision forces the argument exactly once (direct and self-composition), and a lazy identitymapdoes not force unused elements (no spurious trace).Result
No behavior change — the audit found the optimizations sound; these tests lock in their semantics so the aggressive elisions can't silently regress. Full JVM test suite green (EvaluatorTests runs both the old and new evaluator variants).
Addresses #815.