diff --git a/play-json/jvm/src/test/scala-3/play/api/libs/json/JsonMemoryFootprintSpec.scala b/play-json/jvm/src/test/scala-3/play/api/libs/json/JsonMemoryFootprintSpec.scala index c9166b7a..f5c259b5 100644 --- a/play-json/jvm/src/test/scala-3/play/api/libs/json/JsonMemoryFootprintSpec.scala +++ b/play-json/jvm/src/test/scala-3/play/api/libs/json/JsonMemoryFootprintSpec.scala @@ -55,6 +55,17 @@ class JsonMemoryFootprintSpec extends AnyFreeSpec { } } + "Json.newBuilder" - { + "takes the same memory as JsObject(fields)" in { + 1.to(20).foreach { n => + val fields = 0.until(n).map(i => s"$i" -> JsTrue) + val built = footprint(fields.foldLeft(Json.newBuilder)((b, f) => b += (f._1 -> f._2)).result()) + val oneGo = footprint(JsObject(fields)) + withClue(s"$n fields:")(assert(built > 0 && built === oneGo)) + } + } + } + // By-name: each layout is diffed against a second instance, so shared singletons don't count. private def footprint(a: => JsValue): Long = GraphLayout.parseInstance(a).subtract(GraphLayout.parseInstance(a)).totalSize() diff --git a/play-json/shared/src/main/scala-2.13+/play/api/libs/json/ImmutableLinkedHashMap.scala b/play-json/shared/src/main/scala-2.13+/play/api/libs/json/ImmutableLinkedHashMap.scala index 78477326..a04ac2e6 100644 --- a/play-json/shared/src/main/scala-2.13+/play/api/libs/json/ImmutableLinkedHashMap.scala +++ b/play-json/shared/src/main/scala-2.13+/play/api/libs/json/ImmutableLinkedHashMap.scala @@ -79,16 +79,34 @@ private[json] object ImmutableLinkedHashMap extends MapFactory[Map] { override def newBuilder[A, B]: mutable.Builder[(A, B), Map[A, B]] = new mutable.Builder[(A, B), Map[A, B]] { private var lhm = new JLinkedHashMap[A, B](0) - override def clear(): Unit = lhm.clear() + // Whether lhm backs a map returned by result(), so it must be copied before any change. + private var aliased = false + + override def clear(): Unit = { + if (aliased) { + lhm = new JLinkedHashMap[A, B](0) + aliased = false + } else lhm.clear() + } override def sizeHint(size: Int): Unit = if (size > 0 && lhm.isEmpty) lhm = new JLinkedHashMap[A, B](size) + override def knownSize: Int = lhm.size() + override def result(): Map[A, B] = { if (lhm.isEmpty) empty - else new ImmutableLinkedHashMap(lhm) + else { + aliased = true + new ImmutableLinkedHashMap(lhm) + } } override def addOne(elem: (A, B)): this.type = { + if (aliased) { + lhm = new JLinkedHashMap[A, B](lhm) + aliased = false + } + lhm.put(elem._1, elem._2) this } diff --git a/play-json/shared/src/main/scala-2.13+/play/api/libs/json/JsObjectBuilder.scala b/play-json/shared/src/main/scala-2.13+/play/api/libs/json/JsObjectBuilder.scala index a6e98437..769500ef 100644 --- a/play-json/shared/src/main/scala-2.13+/play/api/libs/json/JsObjectBuilder.scala +++ b/play-json/shared/src/main/scala-2.13+/play/api/libs/json/JsObjectBuilder.scala @@ -8,12 +8,14 @@ import scala.collection.mutable.{ Builder => MBuilder } private[json] final class JsObjectBuilder extends MBuilder[(String, Json.JsValueWrapper), JsObject] { - private val fs = Map.newBuilder[String, JsValue] + // Keeps the fields in insertion order, as JsObject.apply does. + private val fs = ImmutableLinkedHashMap.newBuilder[String, JsValue] - def addOne(elem: (String, Json.JsValueWrapper)): this.type = { - val (name, wrapped) = elem + def addOne(elem: (String, Json.JsValueWrapper)): this.type = append(elem._1, elem._2) - fs += (name -> Json.unwrap(wrapped)) + // Adds a field without the caller allocating a tuple. + def append(key: String, value: Json.JsValueWrapper): this.type = { + fs += (key -> Json.unwrap(value)) this } diff --git a/play-json/shared/src/main/scala-2.13-/play/api/libs/json/JsObjectBuilder.scala b/play-json/shared/src/main/scala-2.13-/play/api/libs/json/JsObjectBuilder.scala index 70c41d61..342273e0 100644 --- a/play-json/shared/src/main/scala-2.13-/play/api/libs/json/JsObjectBuilder.scala +++ b/play-json/shared/src/main/scala-2.13-/play/api/libs/json/JsObjectBuilder.scala @@ -8,12 +8,14 @@ import scala.collection.mutable.{ Builder => MBuilder } private[json] final class JsObjectBuilder extends MBuilder[(String, Json.JsValueWrapper), JsObject] { - private val fs = Map.newBuilder[String, JsValue] + // Keeps the fields in insertion order, as JsObject.apply does. + private val fs = ImmutableLinkedHashMap.newBuilder[String, JsValue] - def +=(elem: (String, Json.JsValueWrapper)): this.type = { - val (name, wrapped) = elem + def +=(elem: (String, Json.JsValueWrapper)): this.type = append(elem._1, elem._2) - fs += (name -> Json.unwrap(wrapped)) + // Adds a field without the caller allocating a tuple. + def append(key: String, value: Json.JsValueWrapper): this.type = { + fs += (key -> Json.unwrap(value)) this } diff --git a/play-json/shared/src/test/scala/play/api/libs/json/JsonSharedSpec.scala b/play-json/shared/src/test/scala/play/api/libs/json/JsonSharedSpec.scala index 62cb66be..05ee3a04 100644 --- a/play-json/shared/src/test/scala/play/api/libs/json/JsonSharedSpec.scala +++ b/play-json/shared/src/test/scala/play/api/libs/json/JsonSharedSpec.scala @@ -174,6 +174,54 @@ class JsonSharedSpec ) } + "create object using builder, keeping the field order" in { + // more fields than the specialised small immutable Maps hold + val fields = 1.to(20).map(i => s"f$i" -> JsNumber(i)) + val builder = Json.newBuilder + + fields.foreach { case (k, v) => builder += (k -> v) } + builder += ("f3" -> 30) + + val expected = JsObject(fields.updated(2, "f3" -> JsNumber(30))) + + Json.stringify(builder.result()).mustEqual(Json.stringify(expected)) + } + + "not change a built object when the builder is reused" in { + val builder = Json.newBuilder + + builder += ("a" -> 1) + builder += ("b" -> 2) + + val first = builder.result() + + builder += ("c" -> 3) + + val second = builder.result() + + builder.clear() + builder += ("d" -> 4) + + Json.stringify(first).mustEqual("""{"a":1,"b":2}""") + Json.stringify(second).mustEqual("""{"a":1,"b":2,"c":3}""") + Json.stringify(builder.result()).mustEqual("""{"d":4}""") + } + + "create object using builder append, mixed with +=" in { + val builder = new JsObjectBuilder() + + builder.append("a", 1) + builder += ("b" -> "two") + builder.append("c", Json.arr(3)) + + val first = builder.result() + + builder.append("a", 10) + + Json.stringify(first).mustEqual("""{"a":1,"b":"two","c":[3]}""") + Json.stringify(builder.result()).mustEqual("""{"a":10,"b":"two","c":[3]}""") + } + "convert to a byte array containing the UTF-8 representation" in json { js => val json = js.parse(""" |{