switch to scala 2.12 and add scaffolding for scala.js #1426
Conversation
.gitignore
Outdated
| @@ -1,2 +1,3 @@ | |||
| tags | |||
| target | |||
| .DS_Store | |||
There was a problem hiding this comment.
That should not be here, you can include such OS specific exclude in your global gitignore, ty
There was a problem hiding this comment.
Sounds good. Should I remove tags as well, while I'm in there?
There was a problem hiding this comment.
I deleted everything but target.
| scalacOptions ++= Seq("-feature","-deprecation", "-Xlint", "-language:higherKinds", | ||
| "-Ydelambdafy:method", "-Ypartial-unification", "-target:jvm-1.8"), | ||
| "-Ydelambdafy:method", "-Ypartial-unification", "-target:jvm-1.8", "-opt:l:project", |
There was a problem hiding this comment.
I find suspicious that -opt:l:project is required here, what is the impact on performance?
Inlining is known to have cause issues in the best in users library/projects.
There was a problem hiding this comment.
It's definitely measurable. However, I can manually inline the critical section if necessary, which will achieve most of the gains. The optimizer will only inline methods so annotated (@inline), and it looks more carefully engineered than 2.11.x.
Let me know how to handle it.
There was a problem hiding this comment.
It would interesting to have opinion of @retronym on such thing, as he knows how such a library is crafted and he is probably well aware of the current state of the inliner.
I don't feel qualified enough to make a decision here, I'm just pushing on the safe side due to bad memories about inlining... But it would be a shame to miss such improvement if they are now properly implemented... might be worth trying initially... and can go back later...
Thanks Kenji for the pointer!
There was a problem hiding this comment.
I left the inline pragma for now. However, in the future, if we decide to remove it, I can manually inline critical section code.
There was a problem hiding this comment.
it looks like opt:l:project is now deprecated: scala/scala#5964
build.sbt
Outdated
| @@ -1,28 +1,57 @@ | |||
| import org.scalajs.sbtplugin.ScalaJSPlugin | |||
| import org.scalajs.sbtplugin.ScalaJSPlugin.autoImport._ | |||
There was a problem hiding this comment.
maybe these imports unnecessary
There was a problem hiding this comment.
I deleted them, turns out they were not required.
|
Great effort BTW, I wish I did start this way. |
|
@aloiscochard @xuwei-k All right, this is ready for merge. |
|
@aloiscochard @xuwei-k Anything else I should change before this gets merged? |
|
LGTM! great stuff :) |
|
@aloiscochard Would you mind merging? I don't have permissions. Thanks! |
|
@jbgi Great. So now you have to inline in two places: the code, and the build file. 🙄 😆 I think in my next PR I'll just manually inline... |
A bunch of prep work to ease the painful situation I'm in presently with scalaz.effect.