search

amnaredo / test

0 stars 0 forks source link

ReadWriter bundle size performance #277

Open amnaredo opened 3 years ago

amnaredo commented 3 years ago

Hello,

In Scala.js, ReadWriter-s created by macroRW take up quite a bit of space. For example, the macroRW for a small case class like this:

case class UserAccount(
  id: String,
  email: String,
  name: String,
  createdAt: Double,
  updatedAt: Double
)

takes up approx 3KB (Scala.js 1.x, Scala 2.13, fullOpt, es2015 on, pre-gzip). A sealed trait hierarchy consisting of 8 case classes with 4-6 fields each takes up 27KB. This adds up really fast to the point where upickle ReadWriters use up more than 10% of my total bundle size. Upickle the library itself takes 78KB on top of that, but that's fine by me because it's static. On the other hand, ReadWriter size grows very fast, every new shared model that I add, most of the size is coming from its ReadWriter.

My methodology for estimating ReadWriters' contribution to bundle size: feed the generated js bundle file to source-map-explorer, make sure source maps are available too. This gives a breakdown of how much each of my Scala classes/files takes up space. Observe the reported size for UserAccount (for example, a class that has implicit val rw: ReadWriter[UserAccount] = macroRW defined in its companion object). Then, replace macroRW with ??? and generate & analyze the bundle again. Observe the new reported size for UserAccount and calculate the difference. I believe this tells me the size of the generated ReadWriter code for this macroRW, free of any effect of DCE on upickle's base types because those live in your upickle package, and their size is reported separately (and does change slightly too, consistent with the idea).

My main concern is with macroRW ReadWriter-s because this is what I use for the most part. Manually created ReadWriters appear to take less space, but are more cumbersome to write, and allow for mistakes.

I guess my question is, are improvements in bundle size possible for macroRW-generated ReadWriter-s? And if so, do you plan to take that on some time? I have zero experience with macros so it's hard for me to tell just by looking at the code whether it's possible or how hard it would be, but I expect that such things don't come easy. If there are good ways to reduce upickle-related bundle size other than writing ReadWriter-s manually, would be good to know as well. ID: 324 Original Author: raquo

amnaredo commented 3 years ago

The code generation all happens here:

It is not currently optimized for code size. There are definitely things you can do to reduce the size of generated code, e.g. by moving commonly-generated code into helper methods (e.g. here, here), and there's even more stuff you can do if you're willing to accept some performance hit (e.g. converting local variables into dictionaries here). There is already a significant amount of code in the CaseW and CaseR traits/classes, so moving more logic there is a reasonable thing to do

I have no plans to work on this, but feel free to poke around that code and see how much of an improvement you can squeeze out of it. The non-performance-affecting changes would be welcome as PRs directly to the macro code, and the performance-affecting changes would be welcome as a separate macro people could opt-into (e.g. using compactMacroRW rather than macroRW) Original Author: lihaoyi

amnaredo commented 3 years ago

@raquo could you try building uPickle from this PR and see if it makes any difference? https://github.com/com-lihaoyi/upickle/pull/335 Original Author: lihaoyi

amnaredo commented 3 years ago

👍 I guess I'll close this seeing that the easy wins are done in #335. Original Author: raquo