Optimize UnwrapArray parser by caching JValue ClassTag #547
Conversation
| @@ -81,6 +82,7 @@ sealed abstract class JValue { | |||
|
|
|||
| object JValue { | |||
| implicit val facade: Facade[JValue] = JawnFacade | |||
| implicit val classTag: ClassTag[JValue] = ClassTag(classOf[JValue]) | |||
There was a problem hiding this comment.
nit: I wonder if it's worth making this lazy?
There was a problem hiding this comment.
What are you trying to optimize? The memory consumption?
On the other hand the UnwrapArray parser would have to repeatedly go through the synchronized access.
There was a problem hiding this comment.
Load time. Companion object gets initialized the moment the type gets used so I thought it might be worth it to defer reflection call until the code path hits it.
There was a problem hiding this comment.
Sounds reasonable 👍
I made it lazy.
| @@ -81,6 +82,7 @@ sealed abstract class JValue { | |||
|
|
|||
| object JValue { | |||
| implicit val facade: Facade[JValue] = JawnFacade | |||
| implicit lazy val classTag: ClassTag[JValue] = ClassTag(classOf[JValue]) | |||
| @@ -81,6 +82,7 @@ sealed abstract class JValue { | |||
|
|
|||
| object JValue { | |||
| implicit val facade: Facade[JValue] = JawnFacade | |||
| implicit lazy val classTag: ClassTag[JValue] = ClassTag(classOf[JValue]) | |||
There was a problem hiding this comment.
Complete nitpick, but does this need to be public? Seems like an implementation detail.
There was a problem hiding this comment.
Totally legit nitpick, I made it private in c61b951
I had to add a scope to the private field otherwise the compilation fails with
[error] /home/agourlay/Workspace/jawn/ast/shared/src/main/scala/jawn/ast/JawnFacade.scala:56:55: lazy value classTag in object JValue cannot be accessed in object org.typelevel.jawn.ast.JValue
[error] Access to protected lazy value classTag not permitted because
[error] enclosing <$anon: org.typelevel.jawn.FContext.NoIndexFContext[org.typelevel.jawn.ast.JValue]> is not a subclass of
[error] object JValue in package ast where target is defined
[error] def finish(): JValue = JArray(vs.toArray(JValue.classTag))
| @@ -81,6 +82,7 @@ sealed abstract class JValue { | |||
|
|
|||
| object JValue { | |||
| implicit val facade: Facade[JValue] = JawnFacade | |||
| implicit private[jawn] lazy val classTag: ClassTag[JValue] = ClassTag(classOf[JValue]) | |||
There was a problem hiding this comment.
note that lazy adds some additional synchronization overhead compared to a vanilla val, which is why we have unsafe lazies in Scala 3.
There was a problem hiding this comment.
I suppose it could be rewritten as a null init and probably squeeze out a few more femtoseconds.
There was a problem hiding this comment.
Re: val vs lazy val, I didn't confirm with a benchmark, but my speculation was that val would regress the load time for users who just see JValue as type signature but do not hit the UnwrapArray parser.
#547 (comment)
There was a problem hiding this comment.
Right, sure :) basically we are trading off a one-time load penalty for a penalty on every access.
|
Thank you very much for the super fast release 👏 I was able to validate that I get a nice performance boost from this change. |
While using the
UnwrapArrayparsing mode, I can see a very large amount ofjava.lang.ClassValuecalls.This accounts for almost 30% of CPU time for very large inputs.
This PR proposes to cache an implicit
ClassTagvalue for theJValuein order to speed things up.I was unfortunately not able to validate the fix as I have issues producing snapshots locally.