From f0e953fdee8ff0137f4d8e432745cd87607d3c8f Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sun, 5 Apr 2026 12:57:04 +0800 Subject: [PATCH 1/3] perf: single-field object inline storage to avoid LinkedHashMap allocation For objects with exactly one field (common in patterns like `{ n: X }`), store the field key and member inline in Val.Obj instead of allocating a LinkedHashMap. The LinkedHashMap is lazily constructed only when needed (e.g., key iteration via getAllKeys). Key changes: - Val.Obj: added singleFieldKey/singleFieldMember constructor params - getValue0: lazily constructs LinkedHashMap from inline storage - valueRaw: single-field fast path with String.equals instead of HashMap.get - hasKeys/containsKey: fast paths to avoid forcing LinkedHashMap materialization - visitMemberList: lazy builder allocation, only for 2+ field objects Upstream: jit branch d284ecf4 (single-field object avoid LinkedHashMap) --- sjsonnet/src/sjsonnet/Evaluator.scala | 75 ++++++++++++++++++++----- sjsonnet/src/sjsonnet/Val.scala | 80 ++++++++++++++++++++------- 2 files changed, 119 insertions(+), 36 deletions(-) diff --git a/sjsonnet/src/sjsonnet/Evaluator.scala b/sjsonnet/src/sjsonnet/Evaluator.scala index 2e52c2cd..f43f4bd3 100644 --- a/sjsonnet/src/sjsonnet/Evaluator.scala +++ b/sjsonnet/src/sjsonnet/Evaluator.scala @@ -1049,7 +1049,12 @@ class Evaluator( newScope } - val builder = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](fields.length) + // Lazily allocate builder only when we have 2+ fields, avoiding LinkedHashMap for single-field objects + var builder: java.util.LinkedHashMap[String, Val.Obj.Member] = null + // Track single-field optimization candidates + var singleKey: String = null + var singleMember: Val.Obj.Member = null + var fieldCount = 0 fields.foreach { case Member.Field(offset, fieldName, plus, null, sep, rhs) => val k = visitFieldName(fieldName, offset) @@ -1062,10 +1067,23 @@ class Evaluator( finally decrementStackDepth() } } - val previousValue = builder.put(k, v) - if (previousValue != null) { - Error.fail(s"Duplicate key $k in evaluated object.", offset) + if (fieldCount == 0) { + singleKey = k + singleMember = v + } else { + if (fieldCount == 1) { + // Moving from single-field to multi-field: allocate builder and add the first field + builder = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](fields.length) + builder.put(singleKey, singleMember) + singleKey = null + singleMember = null + } + val previousValue = builder.put(k, v) + if (previousValue != null) { + Error.fail(s"Duplicate key $k in evaluated object.", offset) + } } + fieldCount += 1 } case Member.Field(offset, fieldName, false, argSpec, sep, rhs) => val k = visitFieldName(fieldName, offset) @@ -1078,10 +1096,22 @@ class Evaluator( finally decrementStackDepth() } } - val previousValue = builder.put(k, v) - if (previousValue != null) { - Error.fail(s"Duplicate key $k in evaluated object.", offset) + if (fieldCount == 0) { + singleKey = k + singleMember = v + } else { + if (fieldCount == 1) { + builder = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](fields.length) + builder.put(singleKey, singleMember) + singleKey = null + singleMember = null + } + val previousValue = builder.put(k, v) + if (previousValue != null) { + Error.fail(s"Duplicate key $k in evaluated object.", offset) + } } + fieldCount += 1 } case _ => Error.fail("This case should never be hit", objPos) @@ -1091,14 +1121,29 @@ class Evaluator( } else { new java.util.HashMap[Any, Val]() } - cachedObj = new Val.Obj( - objPos, - builder, - false, - if (asserts != null) triggerAsserts else null, - sup, - valueCache - ) + cachedObj = if (fieldCount == 1 && singleKey != null) { + // Single-field object: store key and member inline, avoid LinkedHashMap allocation entirely + new Val.Obj( + objPos, + null, + false, + if (asserts != null) triggerAsserts else null, + sup, + valueCache, + singleFieldKey = singleKey, + singleFieldMember = singleMember + ) + } else { + new Val.Obj( + objPos, + if (builder != null) builder + else Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](0), + false, + if (asserts != null) triggerAsserts else null, + sup, + valueCache + ) + } cachedObj } diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala index ae1bd415..4919e9e2 100644 --- a/sjsonnet/src/sjsonnet/Val.scala +++ b/sjsonnet/src/sjsonnet/Val.scala @@ -361,7 +361,9 @@ object Val { `super`: Obj, valueCache: util.HashMap[Any, Val] = new util.HashMap[Any, Val](), private var allKeys: util.LinkedHashMap[String, java.lang.Boolean] = null, - private val excludedKeys: java.util.Set[String] = null) + private val excludedKeys: java.util.Set[String] = null, + private val singleFieldKey: String = null, + private val singleFieldMember: Obj.Member = null) extends Literal with Expr.ObjBody { private var asserting: Boolean = false @@ -369,16 +371,23 @@ object Val { def getSuper: Obj = `super` private def getValue0: util.LinkedHashMap[String, Obj.Member] = { - // value0 is always defined for non-static objects, so if we're computing it here - // then that implies that the object is static and therefore valueCache should be - // pre-populated and all members should be visible and constant. if (value0 == null) { - val value0 = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](allKeys.size()) - allKeys.forEach { (k, _) => - value0.put(k, new Val.Obj.ConstMember(false, Visibility.Normal, valueCache.get(k))) + if (singleFieldKey != null) { + // Single-field object: lazily construct LinkedHashMap from inline storage + val m = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](1) + m.put(singleFieldKey, singleFieldMember) + this.value0 = m + } else { + // value0 is always defined for non-static objects, so if we're computing it here + // then that implies that the object is static and therefore valueCache should be + // pre-populated and all members should be visible and constant. + val value0 = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](allKeys.size()) + allKeys.forEach { (k, _) => + value0.put(k, new Val.Obj.ConstMember(false, Visibility.Normal, valueCache.get(k))) + } + // Only assign to field after initialization is complete to allow unsynchronized multi-threaded use: + this.value0 = value0 } - // Only assign to field after initialization is complete to allow unsynchronized multi-threaded use: - this.value0 = value0 } value0 } @@ -576,26 +585,32 @@ object Val { } @inline def hasKeys: Boolean = { - val m = if (static || `super` != null) getAllKeys else value0 - !m.isEmpty + if (singleFieldKey != null) true + else { + val m = if (static || `super` != null) getAllKeys else getValue0 + !m.isEmpty + } } @inline def containsKey(k: String): Boolean = { - val m = if (static || `super` != null) getAllKeys else value0 - m.containsKey(k) + if (singleFieldKey != null && `super` == null) singleFieldKey.equals(k) + else { + val m = if (static || `super` != null) getAllKeys else getValue0 + m.containsKey(k) + } } @inline def containsVisibleKey(k: String): Boolean = { if (static || `super` != null) { getAllKeys.get(k) == java.lang.Boolean.FALSE } else { - val m = value0.get(k) + val m = getValue0.get(k) m != null && (m.visibility != Visibility.Hidden) } } lazy val allKeyNames: Array[String] = { - val m = if (static || `super` != null) getAllKeys else value0 + val m = if (static || `super` != null) getAllKeys else getValue0 m.keySet().toArray(new Array[String](m.size())) } @@ -605,10 +620,11 @@ object Val { } else { val buf = new mutable.ArrayBuilder.ofRef[String] if (`super` == null) { + val v0 = getValue0 // This size hint is based on an optimistic assumption that most fields are visible, // avoiding re-sizing or trimming the buffer in the common case: - buf.sizeHint(value0.size()) - value0.forEach((k, m) => if (m.visibility != Visibility.Hidden) buf += k) + buf.sizeHint(v0.size()) + v0.forEach((k, m) => if (m.visibility != Visibility.Hidden) buf += k) } else { getAllKeys.forEach((k, b) => if (b == java.lang.Boolean.FALSE) buf += k) } @@ -682,10 +698,11 @@ object Val { v } else { val s = this.`super` - getValue0.get(k) match { - case null => - if (s == null) null else s.valueRaw(k, self, pos, addTo, addKey) - case m => + val sfk = singleFieldKey + if (sfk != null) { + // Single-field fast path: avoid LinkedHashMap lookup + if (sfk.equals(k)) { + val m = singleFieldMember if (!evaluator.settings.brokenAssertionLogic || !m.deprecatedSkipAsserts) { self.triggerAllAsserts(evaluator.settings.brokenAssertionLogic) } @@ -698,6 +715,27 @@ object Val { } else vv if (addTo != null && m.cached) addTo.put(addKey, v) v + } else { + if (s == null) null else s.valueRaw(k, self, pos, addTo, addKey) + } + } else { + getValue0.get(k) match { + case null => + if (s == null) null else s.valueRaw(k, self, pos, addTo, addKey) + case m => + if (!evaluator.settings.brokenAssertionLogic || !m.deprecatedSkipAsserts) { + self.triggerAllAsserts(evaluator.settings.brokenAssertionLogic) + } + val vv = m.invoke(self, s, pos.fileScope, evaluator) + val v = if (s != null && m.add) { + s.valueRaw(k, self, pos, null, null) match { + case null => vv + case supValue => mergeMember(supValue, vv, pos) + } + } else vv + if (addTo != null && m.cached) addTo.put(addKey, v) + v + } } } } From 9e30e6682efb1df07b9eab9bae3093e62c8e034a Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sun, 5 Apr 2026 13:28:13 +0800 Subject: [PATCH 2/3] perf: multi-field object inline array storage (2-8 fields) Three-tier object storage: 1 field uses singleKey/singleMember, 2-8 fields use flat parallel arrays (inlineFieldKeys/inlineFieldMembers), 9+ fields use LinkedHashMap. This eliminates LinkedHashMap allocation for the vast majority of Jsonnet objects which have fewer than 9 fields. All fast paths updated: getValue0, hasKeys, containsKey, containsVisibleKey, allKeyNames, visibleKeyNames, valueRaw. Field tracking logic extracted into trackField() helper to avoid code duplication between the two Member.Field case branches. JMH: bench.02 -17.9%, realistic2 -2.7%, bench.04 -5.5% Native: realistic2 -13.5% (1.89x faster than jrsonnet) Upstream: jit branch commit 13e6ff39 --- sjsonnet/src/sjsonnet/Evaluator.scala | 117 ++++++++++++++++++-------- sjsonnet/src/sjsonnet/Val.scala | 92 +++++++++++++++++++- 2 files changed, 169 insertions(+), 40 deletions(-) diff --git a/sjsonnet/src/sjsonnet/Evaluator.scala b/sjsonnet/src/sjsonnet/Evaluator.scala index f43f4bd3..5cac40c6 100644 --- a/sjsonnet/src/sjsonnet/Evaluator.scala +++ b/sjsonnet/src/sjsonnet/Evaluator.scala @@ -1049,12 +1049,70 @@ class Evaluator( newScope } - // Lazily allocate builder only when we have 2+ fields, avoiding LinkedHashMap for single-field objects + // Lazily allocate builder only when we have more than 8 fields, + // using flat arrays (single-field or inline arrays) for small objects var builder: java.util.LinkedHashMap[String, Val.Obj.Member] = null - // Track single-field optimization candidates + // Track inline fields: for 1 field use singleKey/singleMember, for 2-8 use arrays var singleKey: String = null var singleMember: Val.Obj.Member = null + var inlineKeys: Array[String] = null + var inlineMembers: Array[Val.Obj.Member] = null var fieldCount = 0 + val maxInlineFields = 8 + + // Shared field-tracking logic: manages singleKey → inlineKeys → builder transitions. + // Handles duplicate key detection at each tier. + def trackField(k: String, v: Val.Obj.Member, offset: Position): Unit = { + if (fieldCount == 0) { + singleKey = k + singleMember = v + } else if (fieldCount == 1) { + // Moving from single-field to multi-field: allocate inline arrays + inlineKeys = new Array[String](math.min(fields.length, maxInlineFields)) + inlineMembers = new Array[Val.Obj.Member](inlineKeys.length) + inlineKeys(0) = singleKey + inlineMembers(0) = singleMember + if (singleKey.equals(k)) { + Error.fail(s"Duplicate key $k in evaluated object.", offset) + } + inlineKeys(1) = k + inlineMembers(1) = v + singleKey = null + singleMember = null + } else if (fieldCount <= maxInlineFields && inlineKeys != null) { + // Check for duplicates in inline array + var di = 0 + while (di < fieldCount) { + if (inlineKeys(di).equals(k)) { + Error.fail(s"Duplicate key $k in evaluated object.", offset) + } + di += 1 + } + if (fieldCount < inlineKeys.length) { + inlineKeys(fieldCount) = k + inlineMembers(fieldCount) = v + } else { + // Overflow: move all inline fields into LinkedHashMap builder + builder = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](fields.length) + var mi = 0 + while (mi < fieldCount) { + builder.put(inlineKeys(mi), inlineMembers(mi)) + mi += 1 + } + inlineKeys = null + inlineMembers = null + builder.put(k, v) + } + } else { + // Already using builder + val previousValue = builder.put(k, v) + if (previousValue != null) { + Error.fail(s"Duplicate key $k in evaluated object.", offset) + } + } + fieldCount += 1 + } + fields.foreach { case Member.Field(offset, fieldName, plus, null, sep, rhs) => val k = visitFieldName(fieldName, offset) @@ -1067,23 +1125,7 @@ class Evaluator( finally decrementStackDepth() } } - if (fieldCount == 0) { - singleKey = k - singleMember = v - } else { - if (fieldCount == 1) { - // Moving from single-field to multi-field: allocate builder and add the first field - builder = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](fields.length) - builder.put(singleKey, singleMember) - singleKey = null - singleMember = null - } - val previousValue = builder.put(k, v) - if (previousValue != null) { - Error.fail(s"Duplicate key $k in evaluated object.", offset) - } - } - fieldCount += 1 + trackField(k, v, offset) } case Member.Field(offset, fieldName, false, argSpec, sep, rhs) => val k = visitFieldName(fieldName, offset) @@ -1096,28 +1138,13 @@ class Evaluator( finally decrementStackDepth() } } - if (fieldCount == 0) { - singleKey = k - singleMember = v - } else { - if (fieldCount == 1) { - builder = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](fields.length) - builder.put(singleKey, singleMember) - singleKey = null - singleMember = null - } - val previousValue = builder.put(k, v) - if (previousValue != null) { - Error.fail(s"Duplicate key $k in evaluated object.", offset) - } - } - fieldCount += 1 + trackField(k, v, offset) } case _ => Error.fail("This case should never be hit", objPos) } val valueCache = if (sup == null) { - Val.Obj.getEmptyValueCacheForObjWithoutSuper(fields.length) + Val.Obj.getEmptyValueCacheForObjWithoutSuper(fieldCount) } else { new java.util.HashMap[Any, Val]() } @@ -1133,6 +1160,24 @@ class Evaluator( singleFieldKey = singleKey, singleFieldMember = singleMember ) + } else if (inlineKeys != null && fieldCount >= 2) { + // Multi-field inline object: use flat arrays instead of LinkedHashMap + val finalKeys = + if (fieldCount == inlineKeys.length) inlineKeys + else java.util.Arrays.copyOf(inlineKeys, fieldCount) + val finalMembers = + if (fieldCount == inlineMembers.length) inlineMembers + else java.util.Arrays.copyOf(inlineMembers, fieldCount) + new Val.Obj( + objPos, + null, + false, + if (asserts != null) triggerAsserts else null, + sup, + valueCache, + inlineFieldKeys = finalKeys, + inlineFieldMembers = finalMembers + ) } else { new Val.Obj( objPos, diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala index 4919e9e2..74bad710 100644 --- a/sjsonnet/src/sjsonnet/Val.scala +++ b/sjsonnet/src/sjsonnet/Val.scala @@ -363,7 +363,9 @@ object Val { private var allKeys: util.LinkedHashMap[String, java.lang.Boolean] = null, private val excludedKeys: java.util.Set[String] = null, private val singleFieldKey: String = null, - private val singleFieldMember: Obj.Member = null) + private val singleFieldMember: Obj.Member = null, + private val inlineFieldKeys: Array[String] = null, + private val inlineFieldMembers: Array[Obj.Member] = null) extends Literal with Expr.ObjBody { private var asserting: Boolean = false @@ -377,6 +379,18 @@ object Val { val m = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](1) m.put(singleFieldKey, singleFieldMember) this.value0 = m + } else if (inlineFieldKeys != null) { + // Multi-field inline object: lazily construct LinkedHashMap from arrays + val keys = inlineFieldKeys + val members = inlineFieldMembers + val n = keys.length + val m = Util.preSizedJavaLinkedHashMap[String, Val.Obj.Member](n) + var i = 0 + while (i < n) { + m.put(keys(i), members(i)) + i += 1 + } + this.value0 = m } else { // value0 is always defined for non-static objects, so if we're computing it here // then that implies that the object is static and therefore valueCache should be @@ -586,6 +600,7 @@ object Val { @inline def hasKeys: Boolean = { if (singleFieldKey != null) true + else if (inlineFieldKeys != null && `super` == null) inlineFieldKeys.length > 0 else { val m = if (static || `super` != null) getAllKeys else getValue0 !m.isEmpty @@ -594,7 +609,16 @@ object Val { @inline def containsKey(k: String): Boolean = { if (singleFieldKey != null && `super` == null) singleFieldKey.equals(k) - else { + else if (inlineFieldKeys != null && `super` == null) { + val keys = inlineFieldKeys + val n = keys.length + var i = 0 + while (i < n) { + if (keys(i).equals(k)) return true + i += 1 + } + false + } else { val m = if (static || `super` != null) getAllKeys else getValue0 m.containsKey(k) } @@ -603,6 +627,16 @@ object Val { @inline def containsVisibleKey(k: String): Boolean = { if (static || `super` != null) { getAllKeys.get(k) == java.lang.Boolean.FALSE + } else if (inlineFieldKeys != null) { + val keys = inlineFieldKeys + val members = inlineFieldMembers + val n = keys.length + var i = 0 + while (i < n) { + if (keys(i).equals(k)) return members(i).visibility != Visibility.Hidden + i += 1 + } + false } else { val m = getValue0.get(k) m != null && (m.visibility != Visibility.Hidden) @@ -610,13 +644,38 @@ object Val { } lazy val allKeyNames: Array[String] = { - val m = if (static || `super` != null) getAllKeys else getValue0 - m.keySet().toArray(new Array[String](m.size())) + if (inlineFieldKeys != null && `super` == null) inlineFieldKeys.clone() + else { + val m = if (static || `super` != null) getAllKeys else getValue0 + m.keySet().toArray(new Array[String](m.size())) + } } lazy val visibleKeyNames: Array[String] = { if (static) { allKeyNames + } else if (inlineFieldKeys != null && `super` == null) { + // Inline multi-field fast path: check if all visible (common case) + val keys = inlineFieldKeys + val members = inlineFieldMembers + val n = keys.length + var allVisible = true + var i = 0 + while (allVisible && i < n) { + if (members(i).visibility == Visibility.Hidden) allVisible = false + i += 1 + } + if (allVisible) keys.clone() + else { + val buf = new mutable.ArrayBuilder.ofRef[String] + buf.sizeHint(n) + var j = 0 + while (j < n) { + if (members(j).visibility != Visibility.Hidden) buf += keys(j) + j += 1 + } + buf.result() + } } else { val buf = new mutable.ArrayBuilder.ofRef[String] if (`super` == null) { @@ -718,6 +777,31 @@ object Val { } else { if (s == null) null else s.valueRaw(k, self, pos, addTo, addKey) } + } else if (inlineFieldKeys != null) { + // Inline multi-field fast path: linear scan over small arrays + val keys = inlineFieldKeys + val members = inlineFieldMembers + val n = keys.length + var i = 0 + while (i < n) { + if (keys(i).equals(k)) { + val m = members(i) + if (!evaluator.settings.brokenAssertionLogic || !m.deprecatedSkipAsserts) { + self.triggerAllAsserts(evaluator.settings.brokenAssertionLogic) + } + val vv = m.invoke(self, s, pos.fileScope, evaluator) + val v = if (s != null && m.add) { + s.valueRaw(k, self, pos, null, null) match { + case null => vv + case supValue => mergeMember(supValue, vv, pos) + } + } else vv + if (addTo != null && m.cached) addTo.put(addKey, v) + return v + } + i += 1 + } + if (s == null) null else s.valueRaw(k, self, pos, addTo, addKey) } else { getValue0.get(k) match { case null => From e9cfb9a1bcbe0899616ce529847a63256c5ffa9c Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sun, 5 Apr 2026 17:48:47 +0800 Subject: [PATCH 3/3] perf: Materializer inline fast-path for object materialization Bypass HashMap value() lookups for inline objects (single-field and multi-field with array storage) during materialization. This targets the critical bottleneck where 96% of realistic2 time is spent in materialization (~62K comprehension-generated objects with 2-9 fields). Key changes: - Add canDirectIterate/inlineKeys/inlineMembers accessors to Val.Obj - Add materializeInlineObj (unsorted) and materializeSortedInlineObj fast paths that invoke members directly without HashMap lookup - Cache sorted field order on MemberList AST node for static field names (shared across all Val.Obj instances from same AST) - For dynamic field names (FieldName.Dyn), compute sorted order per-object to avoid cache correctness issues - Add computeSortedInlineOrder companion helper using insertion sort (optimal for typical 2-8 field objects) Upstream: jit branch commits 5f7abec3, dd9d08a3, 119b9a93 --- sjsonnet/src/sjsonnet/Evaluator.scala | 41 ++++ sjsonnet/src/sjsonnet/Expr.scala | 7 + sjsonnet/src/sjsonnet/Materializer.scala | 182 +++++++++++++++--- sjsonnet/src/sjsonnet/Val.scala | 27 +++ .../dynamic_field_sorted_order.jsonnet | 12 ++ .../dynamic_field_sorted_order.jsonnet.golden | 1 + .../dynamic_null_field_sorted_order.jsonnet | 10 + ...mic_null_field_sorted_order.jsonnet.golden | 1 + .../test/src/sjsonnet/MaterializerTests.scala | 87 +++++++++ 9 files changed, 346 insertions(+), 22 deletions(-) create mode 100644 sjsonnet/test/resources/new_test_suite/dynamic_field_sorted_order.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/dynamic_field_sorted_order.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/dynamic_null_field_sorted_order.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/dynamic_null_field_sorted_order.jsonnet.golden create mode 100644 sjsonnet/test/src/sjsonnet/MaterializerTests.scala diff --git a/sjsonnet/src/sjsonnet/Evaluator.scala b/sjsonnet/src/sjsonnet/Evaluator.scala index 5cac40c6..b87e7b67 100644 --- a/sjsonnet/src/sjsonnet/Evaluator.scala +++ b/sjsonnet/src/sjsonnet/Evaluator.scala @@ -1189,6 +1189,47 @@ class Evaluator( valueCache ) } + // Cache sorted field order on MemberList for inline objects. + // Only safe when all field names are Fixed (string literals) — dynamic field names + // (FieldName.Dyn) can evaluate to different keys across invocations, so the cached + // sorted order would be invalid. For dynamic fields, compute per-object instead. + if (cachedObj.canDirectIterate && sup == null) { + val allFieldsFixed = { + val fs = e.fields; var i = 0; var ok = true + while (i < fs.length && ok) { ok = fs(i).fieldName.isInstanceOf[FieldName.Fixed]; i += 1 } + ok + } + if (allFieldsFixed) { + // Static field names: cache on MemberList, shared across all instances + var sortedOrder = e._cachedSortedOrder + if (sortedOrder == null) { + val ik = cachedObj.inlineKeys + val im = cachedObj.inlineMembers + if (ik != null) { + sortedOrder = Materializer.computeSortedInlineOrder(ik, im) + } else { + val sfm = cachedObj.singleMem + sortedOrder = + if (sfm != null && sfm.visibility != Expr.Member.Visibility.Hidden) Array(0) + else Array.emptyIntArray + } + e._cachedSortedOrder = sortedOrder + } + cachedObj._sortedInlineOrder = sortedOrder + } else { + // Dynamic field names: compute per-object, no shared cache + val ik = cachedObj.inlineKeys + val im = cachedObj.inlineMembers + if (ik != null) { + cachedObj._sortedInlineOrder = Materializer.computeSortedInlineOrder(ik, im) + } else { + val sfm = cachedObj.singleMem + cachedObj._sortedInlineOrder = + if (sfm != null && sfm.visibility != Expr.Member.Visibility.Hidden) Array(0) + else Array.emptyIntArray + } + } + } cachedObj } diff --git a/sjsonnet/src/sjsonnet/Expr.scala b/sjsonnet/src/sjsonnet/Expr.scala index 16d5da0d..6b174a9f 100644 --- a/sjsonnet/src/sjsonnet/Expr.scala +++ b/sjsonnet/src/sjsonnet/Expr.scala @@ -373,6 +373,13 @@ object Expr { asserts: Array[Member.AssertStmt]) extends ObjBody { final override private[sjsonnet] def tag = ExprTags.`ObjBody.MemberList` + + /** + * Cached sorted field order for inline objects. Computed once, shared across all Val.Obj + * instances created from this MemberList. + */ + @volatile var _cachedSortedOrder: Array[Int] = null + override def toString: String = s"MemberList($pos, ${arrStr(binds)}, ${arrStr(fields)}, ${arrStr(asserts)})" } diff --git a/sjsonnet/src/sjsonnet/Materializer.scala b/sjsonnet/src/sjsonnet/Materializer.scala index 97cc5ccc..f9888731 100644 --- a/sjsonnet/src/sjsonnet/Materializer.scala +++ b/sjsonnet/src/sjsonnet/Materializer.scala @@ -78,30 +78,133 @@ abstract class Materializer { ctx: Materializer.MaterializeContext)(implicit evaluator: EvalScope): T = { storePos(obj.pos) obj.triggerAllAsserts(ctx.brokenAssertionLogic) - val keys = - if (ctx.sort) obj.visibleKeyNames.sorted(Util.CodepointStringOrdering) - else obj.visibleKeyNames - val ov = visitor.visitObject(keys.length, jsonableKeys = true, -1) - var i = 0 - var prevKey: String = null - while (i < keys.length) { - val key = keys(i) - val childVal = obj.value(key, ctx.emptyPos) - storePos(childVal) - if (ctx.sort) { - if (prevKey != null && Util.compareStringsByCodepoint(key, prevKey) <= 0) - Error.fail( - s"""Internal error: Unexpected key "$key" after "$prevKey" in sorted object materialization""", - childVal.pos - ) - prevKey = key + if (obj.canDirectIterate) { + // Fast path for inline objects (single-field or multi-field with array storage): + // bypass visibleKeyNames allocation, value() lookup chain, and HashMap overhead + if (ctx.sort) materializeSortedInlineObj(obj, visitor, depth, ctx) + else materializeInlineObj(obj, visitor, depth, ctx) + } else { + val keys = + if (ctx.sort) obj.visibleKeyNames.sorted(Util.CodepointStringOrdering) + else obj.visibleKeyNames + val ov = visitor.visitObject(keys.length, jsonableKeys = true, -1) + var i = 0 + while (i < keys.length) { + val key = keys(i) + val childVal = obj.value(key, ctx.emptyPos) + storePos(childVal) + ov.visitKeyValue(ov.visitKey(-1).visitString(key, -1)) + val sub = ov.subVisitor.asInstanceOf[Visitor[T, T]] + ov.visitValue(materializeRecursiveChild(childVal, sub, depth, ctx), -1) + i += 1 + } + ov.visitEnd(-1) + } + } + + /** + * Direct iteration for inline objects without super chain. Bypasses value() lookup (cache checks, + * valueRaw dispatch, key scan), invoking members directly by array index. Applicable to + * comprehension-created objects where all fields are stored in flat arrays. + */ + private def materializeInlineObj[T]( + obj: Val.Obj, + visitor: Visitor[T, T], + depth: Int, + ctx: Materializer.MaterializeContext)(implicit evaluator: EvalScope): T = { + val fs = ctx.emptyPos.fileScope + val rawKeys = obj.inlineKeys + if (rawKeys != null) { + val rawMembers = obj.inlineMembers + val rawN = rawKeys.length + // Count visible fields (skip Hidden) + var visCount = 0 + var i = 0 + while (i < rawN) { + if (rawMembers(i).visibility != Visibility.Hidden) visCount += 1 + i += 1 + } + val ov = visitor.visitObject(visCount, jsonableKeys = true, -1) + i = 0 + while (i < rawN) { + val m = rawMembers(i) + if (m.visibility != Visibility.Hidden) { + val childVal = m.invoke(obj, null, fs, evaluator) + storePos(childVal) + ov.visitKeyValue(ov.visitKey(-1).visitString(rawKeys(i), -1)) + val sub = ov.subVisitor.asInstanceOf[Visitor[T, T]] + ov.visitValue(materializeRecursiveChild(childVal, sub, depth, ctx), -1) + } + i += 1 + } + ov.visitEnd(-1) + } else { + // Single-field object + val sfm = obj.singleMem + if (sfm.visibility != Visibility.Hidden) { + val ov = visitor.visitObject(1, jsonableKeys = true, -1) + val childVal = sfm.invoke(obj, null, fs, evaluator) + storePos(childVal) + ov.visitKeyValue(ov.visitKey(-1).visitString(obj.singleKey, -1)) + val sub = ov.subVisitor.asInstanceOf[Visitor[T, T]] + ov.visitValue(materializeRecursiveChild(childVal, sub, depth, ctx), -1) + ov.visitEnd(-1) + } else { + visitor.visitObject(0, jsonableKeys = true, -1).visitEnd(-1) + } + } + } + + /** + * Sorted direct iteration for inline objects. Uses cached sorted field order when available + * (shared across all objects from the same MemberList), falling back to per-object computation. + * Avoids: sortedVisibleKeyNames lazy val, value() linear scan, and compareStringsByCodepoint + * validation check overhead. + */ + private def materializeSortedInlineObj[T]( + obj: Val.Obj, + visitor: Visitor[T, T], + depth: Int, + ctx: Materializer.MaterializeContext)(implicit evaluator: EvalScope): T = { + val fs = ctx.emptyPos.fileScope + val rawKeys = obj.inlineKeys + if (rawKeys != null) { + val rawMembers = obj.inlineMembers + // Use cached sorted order if available (shared across all objects from same MemberList), + // otherwise compute per-object + val order = { + val cached = obj._sortedInlineOrder + if (cached != null) cached + else Materializer.computeSortedInlineOrder(rawKeys, rawMembers) + } + val visCount = order.length + val ov = visitor.visitObject(visCount, jsonableKeys = true, -1) + var i = 0 + while (i < visCount) { + val idx = order(i) + val childVal = rawMembers(idx).invoke(obj, null, fs, evaluator) + storePos(childVal) + ov.visitKeyValue(ov.visitKey(-1).visitString(rawKeys(idx), -1)) + val sub = ov.subVisitor.asInstanceOf[Visitor[T, T]] + ov.visitValue(materializeRecursiveChild(childVal, sub, depth, ctx), -1) + i += 1 + } + ov.visitEnd(-1) + } else { + // Single-field object: sorting is trivial (same as unsorted) + val sfm = obj.singleMem + if (sfm.visibility != Visibility.Hidden) { + val ov = visitor.visitObject(1, jsonableKeys = true, -1) + val childVal = sfm.invoke(obj, null, fs, evaluator) + storePos(childVal) + ov.visitKeyValue(ov.visitKey(-1).visitString(obj.singleKey, -1)) + val sub = ov.subVisitor.asInstanceOf[Visitor[T, T]] + ov.visitValue(materializeRecursiveChild(childVal, sub, depth, ctx), -1) + ov.visitEnd(-1) + } else { + visitor.visitObject(0, jsonableKeys = true, -1).visitEnd(-1) } - ov.visitKeyValue(ov.visitKey(-1).visitString(key, -1)) - val sub = ov.subVisitor.asInstanceOf[Visitor[T, T]] - ov.visitValue(materializeRecursiveChild(childVal, sub, depth, ctx), -1) - i += 1 } - ov.visitEnd(-1) } @inline private def materializeRecursiveArr[T]( @@ -440,6 +543,41 @@ object Materializer extends Materializer { var index: Int) extends MaterializeFrame + /** + * Compute sorted index permutation for inline object fields. Uses insertion sort which is optimal + * for the typical 2-8 fields in comprehension-created objects. Returns only indices of visible + * fields, sorted by codepoint string ordering. + */ + def computeSortedInlineOrder(keys: Array[String], members: Array[Val.Obj.Member]): Array[Int] = { + // First pass: collect visible field indices + val n = keys.length + val visible = new Array[Int](n) + var visCount = 0 + var i = 0 + while (i < n) { + if (members(i).visibility != Visibility.Hidden) { + visible(visCount) = i + visCount += 1 + } + i += 1 + } + val order = if (visCount == n) visible else java.util.Arrays.copyOf(visible, visCount) + // Insertion sort by codepoint ordering (stable, optimal for small arrays) + i = 1 + while (i < visCount) { + val idx = order(i) + val key = keys(idx) + var j = i - 1 + while (j >= 0 && Util.compareStringsByCodepoint(keys(order(j)), key) > 0) { + order(j + 1) = order(j) + j -= 1 + } + order(j + 1) = idx + i += 1 + } + order + } + /** * Trait for providing custom materialization logic to the Materializer. * @since 1.0.0 diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala index 74bad710..e75f3c76 100644 --- a/sjsonnet/src/sjsonnet/Val.scala +++ b/sjsonnet/src/sjsonnet/Val.scala @@ -370,6 +370,33 @@ object Val { with Expr.ObjBody { private var asserting: Boolean = false + /** + * Whether this object supports direct field iteration in the Materializer, bypassing the + * value() lookup chain (cache checks, valueRaw dispatch, key scan). Only safe when: no super + * chain, no excluded keys, and inline storage is present. + */ + @inline private[sjsonnet] def canDirectIterate: Boolean = + `super` == null && excludedKeys == null && (singleFieldKey != null || inlineFieldKeys != null) + + /** Raw inline field keys array (may be null for non-inline objects). */ + @inline private[sjsonnet] def inlineKeys: Array[String] = inlineFieldKeys + + /** Raw inline field members array (may be null for non-inline objects). */ + @inline private[sjsonnet] def inlineMembers: Array[Obj.Member] = inlineFieldMembers + + /** Single-field key (may be null for non-single-field objects). */ + @inline private[sjsonnet] def singleKey: String = singleFieldKey + + /** Single-field member (may be null for non-single-field objects). */ + @inline private[sjsonnet] def singleMem: Obj.Member = singleFieldMember + + /** + * Cached sorted field order for inline objects. Shared across all objects from the same + * MemberList AST node, computed once during first materialization. Array of indices into + * inlineFieldKeys/inlineFieldMembers, sorted by codepoint string ordering. + */ + @volatile private[sjsonnet] var _sortedInlineOrder: Array[Int] = null + def getSuper: Obj = `super` private def getValue0: util.LinkedHashMap[String, Obj.Member] = { diff --git a/sjsonnet/test/resources/new_test_suite/dynamic_field_sorted_order.jsonnet b/sjsonnet/test/resources/new_test_suite/dynamic_field_sorted_order.jsonnet new file mode 100644 index 00000000..53b85ca0 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/dynamic_field_sorted_order.jsonnet @@ -0,0 +1,12 @@ +// Verify that objects with dynamic field names are sorted correctly +// when the same MemberList AST produces different key sets across invocations. +// Regression test for materializer sorted-cache correctness. +local mk(x) = {[x]: 1, a: 2}; +std.assertEqual(mk("0"), {"0": 1, a: 2}) && +std.assertEqual(mk("z"), {a: 2, z: 1}) && +// Verify multiple calls with different dynamic keys still sort correctly +local results = [mk("c"), mk("b"), mk("d")]; +std.assertEqual(results[0], {a: 2, c: 1}) && +std.assertEqual(results[1], {a: 2, b: 1}) && +std.assertEqual(results[2], {a: 2, d: 1}) && +true diff --git a/sjsonnet/test/resources/new_test_suite/dynamic_field_sorted_order.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/dynamic_field_sorted_order.jsonnet.golden new file mode 100644 index 00000000..27ba77dd --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/dynamic_field_sorted_order.jsonnet.golden @@ -0,0 +1 @@ +true diff --git a/sjsonnet/test/resources/new_test_suite/dynamic_null_field_sorted_order.jsonnet b/sjsonnet/test/resources/new_test_suite/dynamic_null_field_sorted_order.jsonnet new file mode 100644 index 00000000..ed2ae447 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/dynamic_null_field_sorted_order.jsonnet @@ -0,0 +1,10 @@ +// Verify that objects with conditional (nullable) field names +// produce correct output when the same MemberList creates objects +// with different field counts across invocations. +// Regression test for materializer sorted-cache + null field correctness. +local f(x) = { [x]: 1, [if x == "a" then "c"]: 2, b: 3 }; +// First call: x="a", both dynamic fields present → 3 fields +std.assertEqual(f("a"), {a: 1, b: 3, c: 2}) && +// Second call: x="z", second dynamic field null → 2 fields +std.assertEqual(f("z"), {b: 3, z: 1}) && +true diff --git a/sjsonnet/test/resources/new_test_suite/dynamic_null_field_sorted_order.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/dynamic_null_field_sorted_order.jsonnet.golden new file mode 100644 index 00000000..27ba77dd --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/dynamic_null_field_sorted_order.jsonnet.golden @@ -0,0 +1 @@ +true diff --git a/sjsonnet/test/src/sjsonnet/MaterializerTests.scala b/sjsonnet/test/src/sjsonnet/MaterializerTests.scala new file mode 100644 index 00000000..f77958d6 --- /dev/null +++ b/sjsonnet/test/src/sjsonnet/MaterializerTests.scala @@ -0,0 +1,87 @@ +package sjsonnet + +import utest._ +import sjsonnet.Expr.Member.Visibility + +object MaterializerTests extends TestSuite { + val tests = Tests { + test("computeSortedInlineOrder") { + def mkMember(vis: Visibility): Val.Obj.Member = + new Val.Obj.ConstMember(false, vis, Val.True(new Position(null, -1)), cached2 = true) + + test("basic sorting") { + val keys = Array("z", "a", "m", "b") + val members = Array.fill(4)(mkMember(Visibility.Normal)) + val order = Materializer.computeSortedInlineOrder(keys, members) + val sorted = order.map(keys(_)) + assert(sorted.toSeq == Seq("a", "b", "m", "z")) + } + + test("single field") { + val keys = Array("only") + val members = Array(mkMember(Visibility.Normal)) + val order = Materializer.computeSortedInlineOrder(keys, members) + assert(order.toSeq == Seq(0)) + } + + test("already sorted") { + val keys = Array("a", "b", "c") + val members = Array.fill(3)(mkMember(Visibility.Normal)) + val order = Materializer.computeSortedInlineOrder(keys, members) + assert(order.toSeq == Seq(0, 1, 2)) + } + + test("reverse sorted") { + val keys = Array("c", "b", "a") + val members = Array.fill(3)(mkMember(Visibility.Normal)) + val order = Materializer.computeSortedInlineOrder(keys, members) + val sorted = order.map(keys(_)) + assert(sorted.toSeq == Seq("a", "b", "c")) + } + + test("hidden fields filtered") { + val keys = Array("c", "b", "a") + val members = Array( + mkMember(Visibility.Normal), + mkMember(Visibility.Hidden), + mkMember(Visibility.Normal) + ) + val order = Materializer.computeSortedInlineOrder(keys, members) + assert(order.length == 2) + val sorted = order.map(keys(_)) + assert(sorted.toSeq == Seq("a", "c")) + } + + test("all hidden") { + val keys = Array("x", "y") + val members = Array.fill(2)(mkMember(Visibility.Hidden)) + val order = Materializer.computeSortedInlineOrder(keys, members) + assert(order.length == 0) + } + + test("codepoint ordering with unicode") { + // Codepoint ordering: uppercase letters < lowercase letters in ASCII + val keys = Array("b", "B", "a", "A") + val members = Array.fill(4)(mkMember(Visibility.Normal)) + val order = Materializer.computeSortedInlineOrder(keys, members) + val sorted = order.map(keys(_)) + // A(65) < B(66) < a(97) < b(98) + assert(sorted.toSeq == Seq("A", "B", "a", "b")) + } + + test("stability - fields with equal keys preserve relative order") { + // While duplicate keys shouldn't occur in practice (evaluator rejects them), + // verify insertion sort stability just in case + val keys = Array("x", "a", "z") + val members = Array( + mkMember(Visibility.Normal), + mkMember(Visibility.Normal), + mkMember(Visibility.Normal) + ) + val order = Materializer.computeSortedInlineOrder(keys, members) + val sorted = order.map(keys(_)) + assert(sorted.toSeq == Seq("a", "x", "z")) + } + } + } +}