scalajson

scalajson copied to clipboard

In response to https://github.com/mdedetrich/scalajson/issues/17#issuecomment-312140277, we should add JNumber converters to various number formats (since its likely most users will get this wrong)

+1

@axel22 What are your opinions of this PR https://github.com/mdedetrich/scalajson/pull/38?

I like it, would be good to have that! About the ScalaMeter tests, it's possible that you would get a better accuracy by adding a loop inside the benchmarked snippet (to increase the number of iterations).

About the @inline annotations on constants, if you use defs everywhere, it's almost certain that the JIT compiler will inline all invocations to them.

I like it, would be good to have that! About the ScalaMeter tests, it's possible that you would get a better accuracy by adding a loop inside the benchmarked snippet (to increase the number of iterations).

Thanks, I will add more benchmarks + add this inner loop

About the @inline annotations on constants, if you use defs everywhere, it's almost certain that the JIT compiler will inline all invocations to them.

Try, I use @inline for 2 reasons

• This is a cross platform library (so it will target scala-native and it currently targets Scala.js). So I wan't really make sure that it inlines (although I am fairly sure that these projects should inline such code)
• It shows the intent of what is meant to happen, i.e. they really are constants

Makes sense!