Add OptionVal for JSON import fast path#1086
Open
He-Pin wants to merge 1 commit into
Open
Conversation
Motivation: Strict .json imports return an optional parse result so callers can fall back to the Jsonnet parser. The previous Option[(Expr, FileScope)] result allocated a scala.Some on each successful JSON fast-path parse. Modification: Add a small internal OptionVal value class for optional non-null reference values and use it for CachedResolver.parseJsonImport and the Preloader call site. Reject null payloads, add direct OptionVal regression tests, and avoid tuple-pattern destructuring in the preloader fast path so the generated bytecode does not introduce an extra Tuple2 allocation. Result: Successful strict .json imports no longer allocate a scala.Some wrapper while preserving fallback behavior for non-JSON or invalid JSON inputs across evaluator and preloader paths. OptionVal's core semantics are covered by tests, and the preloader JSON fast path keeps the intended lower-allocation shape. References: https://github.com/apache/pekko/blob/main/actor/src/main/scala/org/apache/pekko/util/OptionVal.scala
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
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
CachedResolver.parseJsonImportreturns an optional result so callers can use the strict.jsonparser when it succeeds and fall back to the Jsonnet parser otherwise. The oldOption[(Expr, FileScope)]return allocated ascala.Someon every successful JSON fast-path import.Modification
OptionVal[A <: AnyRef]value class backed by a nullable reference.OptionVal.Nonesentinel for empty results.CachedResolver.parseJsonImportand thePreloadercall site fromOptiontoOptionVal.Result
Successful strict
.jsonimports avoid thescala.Somewrapper allocation while keeping the same behavior for:.jsonimports