Can we use scala-reflect instead of scalap in lift-json?

[Jacek Migdal]

Hey Lift developers,

I am a huge fun of lift-json and I use it a lot. However, there is one thing that I don't like. It depends on scala compiler (via scalap), which mean I have to ship several additional MB.

Googling through this list, showed that it's mostly because of historical reasons. I found a note that it may use scala reflection library once Scala 2.10 is released.

I decided to try to port it from scalap to scala-reflect and managed to pass all of the unit tests:
https://github.com/jakozaur/framework/compare/lift:master...master
[1]

That means I could include scala-reflect (3 MB) instead of scala-compiler and scalap (14 MB +0.8 MB). Moreover, many projects already use scala-reflect in my use cases it means usually 15 MB saved. 

I managed to publish my own package, but would be more than happy if I can contribute it to the project.

Few notes:

1. Any code review/comments are welcomed.
2. This change as it is breaks compatibility with older than 2.10 scala versions
 - we may want to fix it, any help with build magic would be appreciated

3. I need to take a lock to avoid thread-safety issues:
http://docs.scala-lang.org/overviews/reflection/thread-safety.html

Otherwise I get non-deterministic test failures.

4. From philosophical point of view, it seems better to depend on public interface than on internal scalap.

5. Would appreciate any tips how to help you to bring that code to master.


Many thanks,

Jacek


Footnotes:
[1] How I run the tests:
./liftsh
++2.10.0
project lift-json
test

[Antonio Salazar Cardozo]

For what it's worth, my understanding is that TypeRef and scala.reflect in general is still experimental in 2.10, so I'd rather we not move to them yet. I also don't know if this would be a sufficiently small enhancement to pass the requirements for contributions from non-committers. Thoughts from others are welcome here on that aspect.

Regardless I think we should hold off on using scala.reflect, even in Lift 3, until 2.11 stabilizes it, assuming 2.11 does.

If I may ask, what's the concern of including an additional 15MB in a WAR file that is deployed to a web server?

Thanks,

Antonio

[Jacek Migdal]

Thank you so much for your reply.

By the way, I realized that if we also use scala reflect instead of Paranamer, lift-json extraction will likely work in Scala REPL. This would be really handy and it's top issue in lift-json FAQ.

Additional 15 MB in every assembly is a problem, because:
- build time is worse (especially if you include upload time and do that in continuous way)
- deploying takes longer
- jvm startup time is also a bit worse

Each of this performance hit is rather a small one, but multiplying it be a huge number of machines and jvms it looks like something worth optimizing. That changed itself, decreased the total size of assemblies by about 15% (I include all of our static resources, it takes much more if I count just jars).

Moreover, I am not the only one experiencing this issue, some links to previous discussions:
1. "Lift JSON uses scalap to parse primitive information from parameterized types. This
code path will be replaced after Scala 2.10 is released. I hope we can drop all 
dependecies (paranamer too) of Lift JSON after we have reimplemented
reflection stuff with scala's forthcoming reflection AP" by Jonni Freeman
https://groups.google.com/forum/#!topic/liftweb/uAT3j9_PZYQ/discussion

2. "lift-json needs to use scalap" ... "seems a bit untidy to have to drag in the whole
compiler for this purpose" ... "reflection library will come along" by Peter Brant 
https://groups.google.com/d/msg/liftweb/L2_92-XW4t0/e-XXu5ME0O0J

3. https://groups.google.com/forum/#!msg/liftweb/tZ3nJkfXZSk/yAOOKOiiNxsJ

...

I see you point about scala-reflect being marked as a experimental. However, in my opinion in it its current form it's way better than scalap.

I believe the root reason why it's "experimental" is that Scala developers want to have more flexibility and they won't to be able to change some API without deprecating it, butt maintaining forever. It is an official Typesafe library, so likely from business point of view that also makes more sense. 

Contrary to that scalap is maintained by community and doesn't provide any guarantees about compatibility. Even in lift-json there was some workaround b/c internal compiler representation changed at some point. Also I found some previous discussions about issues which are caused about limitations of scalp. E.g.:

https://groups.google.com/d/msg/liftweb/6jT2LZHvS3s/WDCsbV-4ZWAJ

So in my eyes scala-reflect is fairly mature official Typesafe library, with some warning that is not as stable as rest of compiler.

I would really love to involve more in lift project, starting from that issue. I read the wiki, but it seems that it would be hard without being a committer. I haven't found any information on how to became one, but would be happy to do more coding if the community would support that change. Even if you don't share my optimization views, I believe that fixing lift-json in REPL might be interesting value proposition.

Many thanks,
Jacek

[Antonio Salazar Cardozo]

On Saturday, November 30, 2013 9:33:04 PM UTC-5, Jacek Migdal wrote:

Thank you so much for your reply.

By the way, I realized that if we also use scala reflect instead of Paranamer, lift-json extraction will likely work in Scala REPL. This would be really handy and it's top issue in lift-json FAQ.

I'm definitely not denying there are advantages to using Scala's reflection primitives :)

 

Additional 15 MB in every assembly is a problem, because:
- build time is worse (especially if you include upload time and do that in continuous way)
- deploying takes longer
- jvm startup time is also a bit worse

Each of this performance hit is rather a small one, but multiplying it be a huge number of machines and jvms it looks like something worth optimizing. That changed itself, decreased the total size of assemblies by about 15% (I include all of our static resources, it takes much more if I count just jars).

So you're having an issue with these times that is significantly contributed to by the 15MB? This is more me trying to understand whether that aspect is truly a core problem for you, so we can see if perhaps someone can suggest a solution that helps you save some of that somehow.

 

Moreover, I am not the only one experiencing this issue, some links to previous discussions:

[SNIP]

Again, not denying there aren't advantages.

 

I see you point about scala-reflect being marked as a experimental. However, in my opinion in it its current form it's way better than scalap.

…

So in my eyes scala-reflect is fairly mature official Typesafe library, with some warning that is not as stable as rest of compiler.

Better doesn't matter in this case. Lift has a history of staying very stable and we generally try not to depend on stuff that's likely to change under us in a source-unsafe manner between Scala releases if at all possible. This is particularly true of areas where the committer who is responsible for that corner of Lift doesn't have as much time for that corner as they used to, as is the case with lift-json. This conservatism has served us well in the past, and means that we can maintain a certain degree of stability for our users, as well. Plus, it saves us nasty cross-version-compile headaches and thus simplifies the release process. Please keep in mind that we're purely volunteer-time-based, so all of these times add up to a significantly larger impact than if this were a project with full-time developers.

Moreover, it unfortunately doesn't matter if scala-reflect is mature in your eyes. In the eyes of its release managers, it's experimental. That means they make no guarantees whatsoever about the API, and we're setting ourselves up for a double-rewrite for no reason.

Additionally, lift-json right now works. There are disadvantages in the resulting WAR size, but I suspect those could be mitigated via release process tweaks (I don't know this for sure, but this is why I want to ascertain whether things like continuous build deploys and such are your concern, so that we can see if someone with more experience in release management has some ideas on how they might be dealt with).

I don't doubt we all want to move to scala-reflect, but my vote is that until it's considered to have a stable API, that shouldn't happen.

 

I would really love to involve more in lift project, starting from that issue. I read the wiki, but it seems that it would be hard without being a committer. I haven't found any information on how to became one, but would be happy to do more coding if the community would support that change. Even if you don't share my optimization views, I believe that fixing lift-json in REPL might be interesting value proposition.

Regarding becoming a committer, the best you can do is be involved in the community here and answer questions and help those who need help. Some particularly helpful members of the community may from time to time be asked if they want to become committers.

Thanks,

Antonio

[OvermindDL1]

For note, I use lifts json in a non-lift project, without including the Scala compiler lib then my distribution size would fall from over 60 megs to about 45 megs, and averaging over 600k downloads a month on the slow months (over 2 million downloads on a new release month) would save me a *lot* of bandwidth, I did not even know scalas compiler could be taken out of it. I use the compiler lib for something else too but I could easily remove that dependency.   Thanks for your branch of that dependency removed, as I use 2.10 I will give it a try, but I would love this merged into the main repo for mavenization purposes.

TL/DR: I have a package with Scala compiler because of lift json, fixing this would save me ~15megs per download over near a million downloads of this project per month.

--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code
 
---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Antonio Salazar Cardozo]

On Sunday, December 1, 2013 5:27:21 PM UTC-5, Gabriel Robertson wrote:

For note, I use lifts json in a non-lift project, without including the Scala compiler lib then my distribution size would fall from over 60 megs to about 45 megs, and averaging over 600k downloads a month on the slow months (over 2 million downloads on a new release month) would save me a *lot* of bandwidth, I did not even know scalas compiler could be taken out of it. I use the compiler lib for something else too but I could easily remove that dependency.   Thanks for your branch of that dependency removed, as I use 2.10 I will give it a try, but I would love this merged into the main repo for mavenization purposes.

TL/DR: I have a package with Scala compiler because of lift json, fixing this would save me ~15megs per download over near a million downloads of this project per month.

Thanks for the informative heads up. It doesn't change my opinion since that opinion was ultimately rooted in API stability, but it's important that we be aware of folks who are feeling the pain of that code size difference.

Thanks,

Antonio

[Diego Medina]

I wanted to add my 2 cents, which go long the lines of what Antonio said.

If we go ahead and make these changes, and around scala 2.11 (or 2.12, etc)  they change the API, we would be in a situation where we will need different source files for Lift version compiled for Scala 2.10, vs 2.11. And get into sbt land to see how to do that, etc (while not impossible, it would add to the time we spend on Lift, and we all do Lift work on our free time, and fixing broken builds isn't that excited.

When we said on the mailing list that once scala 2.10 came up with a new api for reflection, we didn't know they would go ahead and label it "experimental".

All that being said, the size of an app your clients are downloading or the time your server takes to upload your war are real, but the cost of fixing it now could result in us not being able to release Lift for future versions of Lift, or at least it would add a huge delay on getting it ready.

Thanks

  Diego

--

--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code
 
---
You received this message because you are subscribed to the Google Groups "Lift" group.
To unsubscribe from this group and stop receiving emails from it, send an email to liftweb+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--

Diego Medina
Lift/Scala consultant
di...@fmpwizard.com
http://fmpwizard.telegr.am

[Jacek Migdal]

Than you for your opinion and time .

It seems although my concerns are valid (larges size, lift-json not
working in REPL or when case class are in different places) the cost
of fixing that in current version is too high for the community.
However, if there is any way I could have positive impact I would be
glad to help. E.g. maybe we could move ScalaSig class to common
library and then for those which issue is important can just override
that utility.

I would be happy to implement that change including modifying sbt
scripts, but I realize that it will also create some burden for the
lift developers. lift-json is so awesome library that it would be
pleasure if I could pay you back some time I gained ;).

Jacek

Btw, some background why this issue is more important for me. We use
maven with parallel modules. This means that most of our build runs
concurrent (compilation, testing), but due to limitations of some
maven plugin creating assemblies/wars is fully sequential. This is a
bottleneck for us and it took majority of total build time in some
configurations. The time of creating assemblies is linear to total
size so I look for easy wins to decrease it size. scala-compiler is
single largest offender, so I though fixing that would be a win for us
and I could help lift community at the same time.

> You received this message because you are subscribed to a topic in the
> Google Groups "Lift" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/liftweb/VRdhj16NKo8/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to

[David Whittaker]

Diego / Antonio,

I completely agree that API stability is a critical concern here.  I think it's important to keep in mind though that using scalap to reflect on Scala members is not supported or stable either.  I've written code that is very similar to Joni's in Squeryl and changes between Scala 2.9 and Scala 2.10 introduced class incompatibilities in scalap that required some ugly workarounds.  As long as Lift 2 supports Scala 2.9, I think it's reasonable to continue to rely on scalap.  For Lift 3 though, even though the Scala reflection API is considered experimental, I think the fact that it even *has* a public API, which scalap does not, makes it a better candidate.  My own 2 cents.

[Antonio Salazar Cardozo]

That's a very fair point. That said, again, the greatest weight in favor of what's there now is that it works. As such, I would say if the Scala folks break scalap (not sure if we already had to deal with such a break in 2.9->2.10, I just know things have been working in my experience), the right thing would be to rewrite for scala reflect. While things are still working, though, I think we can wait for a stable API for scala reflect.

Thanks,

Antonio