Pull request: the new reflection

145 views
Skip to first unread message

Eugene Burmako

unread,
Jun 2, 2012, 1:21:11 PM6/2/12
to scala-internals

Eugene Burmako

unread,
Jun 2, 2012, 1:23:36 PM6/2/12
to scala-internals
I'll start working on scala-reflect.jar using Josh's scripts:
https://github.com/jsuereth/scala/commit/57ad15444bf3e21f8a7c481dd0486262d68a8520
https://github.com/jsuereth/scala/commit/8a7520ac03188ef13ac372b496f992ac65cc6f2c

@Martin. Even after we split out scala-reflect we'll still be able to
debug the performance degradation using the local history we have (my
topic/reflection branch). So I think it'd be useful to factor out
scala-reflect right away, so that the tools have to be adapted only
once.

On Jun 2, 7:21 pm, Eugene Burmako <eugene.burm...@epfl.ch> wrote:
> https://github.com/scala/scala/pull/656

Eugene Burmako

unread,
Jun 2, 2012, 1:28:29 PM6/2/12
to scala-internals
Forgot to mention this explicitly. After this pull request is
accepted, the tools might blow up. We'll need a coordinated effort to
make sure that everything works.

martin odersky

unread,
Jun 2, 2012, 1:40:43 PM6/2/12
to scala-i...@googlegroups.com
On Sat, Jun 2, 2012 at 7:28 PM, Eugene Burmako <eugene....@epfl.ch> wrote:
Forgot to mention this explicitly. After this pull request is
accepted, the tools might blow up. We'll need a coordinated effort to
make sure that everything works.

Yes, indeed. I hope we will not have to do a major operation like this in the foreseeable future. Great that we are ready to be back in trunk, though.

 - Martin

  
On Jun 2, 7:21 pm, Eugene Burmako <eugene.burm...@epfl.ch> wrote:
> https://github.com/scala/scala/pull/656

--
Martin Odersky
Prof., EPFL and Chairman, Typesafe
PSED, 1015 Lausanne, Switzerland
Tel. EPFL: +41 21 693 6863
Tel. Typesafe: +41 21 691 4967

Eugene Burmako

unread,
Jun 2, 2012, 3:58:43 PM6/2/12
to scala-internals
Oops!!

Compiling 107 files to C:\Projects\KeplerUnderRefactoring\build\locker
\classes\reflect

C:\Projects\KeplerUnderRefactoring\src\reflect\scala\reflect\internal
\Positions.scala:6: error: object nsc is not a member of package tools
type Position = scala.tools.nsc.util.Position

^
C:\Projects\KeplerUnderRefactoring\src\reflect\scala\reflect\internal
\Positions.scala:7: error: object nsc is not a member of package tools
val NoPosition = scala.tools.nsc.util.NoPosition

^

On Jun 2, 7:40 pm, martin odersky <martin.oder...@epfl.ch> wrote:
> On Sat, Jun 2, 2012 at 7:28 PM, Eugene Burmako <eugene.burm...@epfl.ch>wrote:
>
> > Forgot to mention this explicitly. After this pull request is
> > accepted, the tools might blow up. We'll need a coordinated effort to
> > make sure that everything works.
>
> > Yes, indeed. I hope we will not have to do a major operation like this in
>
> the foreseeable future. Great that we are ready to be back in trunk, though.
>
>  - Martin
>
> > On Jun 2, 7:21 pm, Eugene Burmako <eugene.burm...@epfl.ch> wrote:
> > >https://github.com/scala/scala/pull/656
>
> --
> Martin Odersky
> Prof., EPFL <http://www.epfl.ch> and Chairman, Typesafe<http://www.typesafe.com>

Eugene Burmako

unread,
Jun 2, 2012, 4:01:54 PM6/2/12
to scala-internals
It seems that we have to move positions to scala-reflect.jar.

I did this as an experiment, and everything seems to be okay.

Namely:
* scala.tools.nsc.util.*Position* =>
scala.reflect.internal.util.*Position*
* scala.tools.nsc.util.*SourceFile* =>
scala.reflect.internal.util.*SourceFile*
* scala.tools.nsc.io.{11 files} (in scala-compiler.jar) =>
scala.tools.nsc.io.{11 files} (in scala-reflect.jar)
* scala.tools.nsc.util.package += 13 aliases

What's your take on this?

Mirco Dotta

unread,
Jun 2, 2012, 4:56:10 PM6/2/12
to scala-i...@googlegroups.com
This would cause pain for us (Scala IDE), and I guess others (scala-refactoring, sbt, ...).

But type aliases and a deprecation path could work.

-- Mirco

Eugene Burmako

unread,
Jun 2, 2012, 5:09:53 PM6/2/12
to scala-i...@googlegroups.com
Why pain? Wouldn't type aliases be enough?

Mirco Dotta

unread,
Jun 2, 2012, 5:12:49 PM6/2/12
to scala-i...@googlegroups.com
> Wouldn't type aliases be enough?

Type aliases would be enough, but it wasn't clear if that was being considered :)

martin odersky

unread,
Jun 2, 2012, 5:13:45 PM6/2/12
to scala-i...@googlegroups.com
Yes, I think Positions in scala-reflect should work OK with the propoer aliases in place.

Cheers

 - Martin

Prof., EPFL and Chairman, Typesafe

Eugene Burmako

unread,
Jun 2, 2012, 5:16:42 PM6/2/12
to <scala-internals@googlegroups.com>
I wonder whether it is possible to conditionally include an artifact
into a maven/sbt build? I mean, include scala-reflect.jar if 2.10.0 or
higher, and not include it (do everything as usual) if pre-2.10.

Mirco Dotta

unread,
Jun 2, 2012, 5:34:55 PM6/2/12
to scala-i...@googlegroups.com
I'n maven you can use profiles, and we have an example already in our build file (because of scala-actors jar in 2.10).


I haven't used sbt enough yet, but I believe you can easily do it.

if (isScala210(scalaVersion)) libraryDependencies += "org.scala" %% "scala-reflection"

Eugene Burmako

unread,
Jun 2, 2012, 8:26:45 PM6/2/12
to scala-internals
Allrighty migration to scala-reflect.jar is complete:

https://github.com/scalamacros/kepler/tree/pullrequest/scalareflectjar

I'm going to sleep now, and if I wake up and find the tests being
green, I'm adding this commit to the pull request.

martin odersky

unread,
Jun 3, 2012, 7:17:24 AM6/3/12
to scala-i...@googlegroups.com
On Sun, Jun 3, 2012 at 2:26 AM, Eugene Burmako <eugene....@epfl.ch> wrote:
Allrighty migration to scala-reflect.jar is complete:

Excellent! 

https://github.com/scalamacros/kepler/tree/pullrequest/scalareflectjar

I'm going to sleep now, and if I wake up and find the tests being
green, I'm adding this commit to the pull request.

Please don't. I think there should be two different pull requests (or at least two different commits). The reason is that
that we we get a better handle to chase down any performance degradations that have accumulated in the reflection branch. Once we change the build structure, it's much harder to compare before/after.

Cheers

 - Martin

Eugene Burmako

unread,
Jun 3, 2012, 7:25:19 AM6/3/12
to scala-i...@googlegroups.com
The reflection pull request is squashed into a single commit. From my experience with macros, it'll be unrealistic to figure something useful out from it. Hence, I suggest we use my topic/reflection for that purpose, since it's much more fine-grained.

And if we use a different branch anyways, then, I think, both reflection and scala-reflect.jar commits can go together right into trunk. What do you think?

martin odersky

unread,
Jun 3, 2012, 8:10:44 AM6/3/12
to scala-i...@googlegroups.com
On Sun, Jun 3, 2012 at 1:25 PM, Eugene Burmako <eugene....@epfl.ch> wrote:
The reflection pull request is squashed into a single commit. From my experience with macros, it'll be unrealistic to figure something useful out from it. Hence, I suggest we use my topic/reflection for that purpose, since it's much more fine-grained.

And if we use a different branch anyways, then, I think, both reflection and scala-reflect.jar commits can go together right into trunk. What do you think?

Yes, but I think they are still two logically separate commits, so should be separate.

 - Martin 

On 3 June 2012 13:17, martin odersky <martin....@epfl.ch> wrote:


On Sun, Jun 3, 2012 at 2:26 AM, Eugene Burmako <eugene....@epfl.ch> wrote:
Allrighty migration to scala-reflect.jar is complete:

Excellent! 

https://github.com/scalamacros/kepler/tree/pullrequest/scalareflectjar

I'm going to sleep now, and if I wake up and find the tests being
green, I'm adding this commit to the pull request.

Please don't. I think there should be two different pull requests (or at least two different commits). The reason is that
that we we get a better handle to chase down any performance degradations that have accumulated in the reflection branch. Once we change the build structure, it's much harder to compare before/after.

Cheers

 - Martin





--
Martin Odersky
Prof., EPFL and Chairman, Typesafe

Eugene Burmako

unread,
Jun 3, 2012, 2:09:50 PM6/3/12
to scala-internals, Martin Odersky, Vlad Ureche, Joshua Suereth
Status update:

* My previous pull request was based on origin/master (which was last
updated in the beginning of May), not upstream/master, so I redid the
pull request: https://github.com/scala/scala/pull/662.
* There's also another pull request that factors out reflection stuff
into scala-reflect.jar: https://github.com/scala/scala/pull/663.

All tests pass on my laptop, so I let Jenkins do his job and verify
that the code is good.

@Vlad. I'd use your guidance in integrating scala-reflect into
scaladoc. So far my stab at it wasn't particularly successful:
https://github.com/scalamacros/kepler/commit/6a76473f2e20d10284e38132469c25dbb28ce587.
I had to replace a bunch of "import reflect.blah" imports with "import
scala.reflect.blah" stuff to make scaladoc work. This possibly
indicates that I'm doing something wrong. Dunno.

@Josh. I have no idea whether scala-reflect will work with Maven. I
built my changes on your previous work (mentioned earlier in this
thread), but I don't know how to test that maven and publishing to
maven work fine.

@All. We have a performance degradation. It is not just "noticeable",
it is serious. I'm talking 30-40%. God knows what's the reason - this
is a month's worth of work, so fixing it will require some serious
digging. It's up to you, guys, to decide whether we can live with that
for now, or whether we want to delay M4 even more.

On Jun 3, 2:10 pm, martin odersky <martin.oder...@epfl.ch> wrote:
> On Sun, Jun 3, 2012 at 1:25 PM, Eugene Burmako <eugene.burm...@epfl.ch>wrote:
>
> > The reflection pull request is squashed into a single commit. From my
> > experience with macros, it'll be unrealistic to figure something useful out
> > from it. Hence, I suggest we use my topic/reflection for that purpose,
> > since it's much more fine-grained.
>
> > And if we use a different branch anyways, then, I think, both reflection
> > and scala-reflect.jar commits can go together right into trunk. What do you
> > think?
>
> Yes, but I think they are still two logically separate commits, so should
> be separate.
>
>  - Martin
>
>
>
>
>
>
>
> > On 3 June 2012 13:17, martin odersky <martin.oder...@epfl.ch> wrote:
>
> >> On Sun, Jun 3, 2012 at 2:26 AM, Eugene Burmako <eugene.burm...@epfl.ch>wrote:
>
> >>> Allrighty migration to scala-reflect.jar is complete:
>
> >> Excellent!
>
> >>>https://github.com/scalamacros/kepler/tree/pullrequest/scalareflectjar
>
> >>> I'm going to sleep now, and if I wake up and find the tests being
> >>> green, I'm adding this commit to the pull request.
>
> >>> Please don't. I think there should be two different pull requests (or at
> >> least two different commits). The reason is that
> >> that we we get a better handle to chase down any performance degradations
> >> that have accumulated in the reflection branch. Once we change the build
> >> structure, it's much harder to compare before/after.
>
> >> Cheers
>
> >>  - Martin
>
> --
> Martin Odersky
> Prof., EPFL <http://www.epfl.ch> and Chairman, Typesafe<http://www.typesafe.com>

Vlad Ureche

unread,
Jun 3, 2012, 3:12:06 PM6/3/12
to Eugene Burmako, scala-internals, Martin Odersky, Joshua Suereth

On Sun, Jun 3, 2012 at 8:09 PM, Eugene Burmako <eugene....@epfl.ch> wrote:

@Vlad. I'd use your guidance in integrating scala-reflect into
scaladoc. So far my stab at it wasn't particularly successful:
https://github.com/scalamacros/kepler/commit/6a76473f2e20d10284e38132469c25dbb28ce587.
I had to replace a bunch of "import reflect.blah" imports with "import
scala.reflect.blah" stuff to make scaladoc work. This possibly
indicates that I'm doing something wrong. Dunno.


Fully-qualified imports are not a scaladoc requirement. I think it might be a classpath-related issue, maybe ant didn't include scala-reflection.jar in scaladoc's classpath. I'll have a look at it tomorrow morning.

Vlad

Mirco Dotta

unread,
Jun 3, 2012, 3:25:14 PM6/3/12
to scala-i...@googlegroups.com
> @All. We have a performance degradation. It is not just "noticeable",
> it is serious. I'm talking 30-40%. God knows what's the reason - this
> is a month's worth of work, so fixing it will require some serious
> digging. It's up to you, guys, to decide whether we can live with that
> for now, or whether we want to delay M4 even more.


Is this regression affecting compilation time? Does this problem occur
only when using reflection?

I'm sorry if I'm asking already answered questions. If that's the case,
would you point me to the existing discussion so that I catch up.

Cheers,
Mirco

Eugene Burmako

unread,
Jun 3, 2012, 3:50:55 PM6/3/12
to scala-i...@googlegroups.com
The regression affects anything that is compiled by the new compiler, not the programs produced by the compiler (to the best of my knowledge).

The changes involved adding a few layers to the compiler cake and rehashing its guts, and, apparently, this adversely affected overall performance.

Mirco Dotta

unread,
Jun 3, 2012, 4:49:32 PM6/3/12
to scala-i...@googlegroups.com
I see, thanks a lot for the info.

It's a tough decision whether to delay or not the milestone release.

Though, my bias would lean toward not releasing. The reason is that M4 is supposed 
to be the last milestone before RC cycle (right?). I fear that releasing M4 with such a bad 
regression would send the wrong message to the community about 2.10 current status.

Either ways, I believe you guys should take an informed decision, i.e., you should first 
understand why compilation time has increased so much, and then decide whether you 
should release as it is or fix the issue.

Just my $0.02

Eugene Burmako

unread,
Jun 3, 2012, 5:16:29 PM6/3/12
to scala-internals
Allrighty thanks! Here's one more crash due to the same reason:
https://scala-webapps.epfl.ch/jenkins/job/scala-checkin-manual/56/console.

On Jun 3, 9:12 pm, Vlad Ureche <vlad.ure...@gmail.com> wrote:
> On Sun, Jun 3, 2012 at 8:09 PM, Eugene Burmako <eugene.burm...@epfl.ch>wrote:
>
>
>
> > @Vlad. I'd use your guidance in integrating scala-reflect into
> > scaladoc. So far my stab at it wasn't particularly successful:
>
> >https://github.com/scalamacros/kepler/commit/6a76473f2e20d10284e38132...
> > .

Adriaan Moors

unread,
Jun 3, 2012, 5:17:14 PM6/3/12
to scala-i...@googlegroups.com
or maybe we should have an M5 -- in 2 weeks, say

Grzegorz Kossakowski

unread,
Jun 3, 2012, 5:28:35 PM6/3/12
to scala-i...@googlegroups.com
On 3 June 2012 23:17, Adriaan Moors <adriaa...@epfl.ch> wrote:
or maybe we should have an M5 -- in 2 weeks, say

+1 and not merge change that causes such a performance hit. That should be general policy that nothing with 30% performance slow down is merged into master.

I think work on patmat justifies having M4 out right now.

PS. Quite soon I plan to start working on a compiler performance testing and that's why I'm so deeply concerned about this particular issue.

--
Grzegorz Kossakowski

Mirco Dotta

unread,
Jun 3, 2012, 6:41:15 PM6/3/12
to scala-i...@googlegroups.com
or maybe we should have an M5 -- in 2 weeks, say

+1 and not merge change that causes such a performance hit. That should be general policy that nothing with 30% performance slow down is merged into master.

I think work on patmat justifies having M4 out right now.

I like it! +1

Ismael Juma

unread,
Jun 3, 2012, 7:25:59 PM6/3/12
to scala-i...@googlegroups.com
On Sun, Jun 3, 2012 at 10:17 PM, Adriaan Moors <adriaa...@epfl.ch> wrote:
or maybe we should have an M5 -- in 2 weeks, say

Sounds like a good plan.

Best,
Ismael

Josh Suereth

unread,
Jun 3, 2012, 9:29:16 PM6/3/12
to scala-i...@googlegroups.com
Yeah, maybe I should just cut M4 tomorrow before too much more time passes.  We need to get what we have out there.

Seth Tisue

unread,
Jun 3, 2012, 9:48:17 PM6/3/12
to scala-i...@googlegroups.com
On Sun, Jun 3, 2012 at 9:29 PM, Josh Suereth <joshua....@gmail.com> wrote:
> Yeah, maybe I should just cut M4 tomorrow before too much more time passes.
>  We need to get what we have out there.

Yes please!!!

--
Seth Tisue | Northwestern University | http://tisue.net
lead developer, NetLogo: http://ccl.northwestern.edu/netlogo/

Erik Osheim

unread,
Jun 4, 2012, 12:56:43 AM6/4/12
to scala-i...@googlegroups.com
On Sun, Jun 03, 2012 at 09:29:16PM -0400, Josh Suereth wrote:
> Yeah, maybe I should just cut M4 tomorrow before too much more time passes.
> We need to get what we have out there.

Agreed!

Eugene Burmako

unread,
Jun 4, 2012, 2:51:26 AM6/4/12
to scala-internals
Doh, at least, give me time until the next Scala meeting - maybe I'll
be able to figure the performance out.

Hubert Plociniczak

unread,
Jun 4, 2012, 3:38:28 AM6/4/12
to scala-i...@googlegroups.com
We still haven't recovered from the original performance drop that was introduced before M3 (+ 40s on quick.comp). It looks now better as it is only +20s but still those problems have to be solved for 2.10 as well. And the more stuff we add to master the harder it becomes to actually figure out what causes it.

So +1 M4, and M5 later from me.

hubert

Eugene Burmako

unread,
Jun 4, 2012, 3:39:41 AM6/4/12
to scala-i...@googlegroups.com
We did recover from that, or, at least, we should. One of my mid-May patches should've taken care of that.

Oh... It's still +20s? Wow, that's weird.

Hubert Plociniczak

unread,
Jun 4, 2012, 3:43:08 AM6/4/12
to scala-i...@googlegroups.com

iulian dragos

unread,
Jun 4, 2012, 5:33:29 AM6/4/12
to scala-i...@googlegroups.com
That commit says:

"1,111 changed files with 14,570 additions and 4,838 deletions."

Now, if you have 10k LOC extra to compile, longer builds are to be
expected, even though the compiler might not be any slower. Toni is
working on a different set of charts, that will plot compilation speed
(time/LOC).

The current benchmarking results are close to useless when trying to
infer performance. They're informative only with regards to CPU load
on chara.

iulian
--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

Eugene Burmako

unread,
Jun 4, 2012, 6:02:28 AM6/4/12
to scala-i...@googlegroups.com
Hmm, so all Jenkins jobs run on the same machine?

iulian dragos

unread,
Jun 4, 2012, 6:18:00 AM6/4/12
to scala-i...@googlegroups.com
On Mon, Jun 4, 2012 at 12:02 PM, Eugene Burmako <eugene....@epfl.ch> wrote:
> Hmm, so all Jenkins jobs run on the same machine?

No, it was a figure of speech. Mr. Jenkins has several slave nodes,
but benchmarks are run in isolation on an otherwise-not used machine.

cheers,
iulian

Hubert Plociniczak

unread,
Jun 4, 2012, 7:07:17 AM6/4/12
to scala-i...@googlegroups.com
On 06/04/2012 11:33 AM, iulian dragos wrote:
> That commit says:
>
> "1,111 changed files with 14,570 additions and 4,838 deletions."
>
> Now, if you have 10k LOC extra to compile, longer builds are to be
> expected, even though the compiler might not be any slower. Toni is
> working on a different set of charts, that will plot compilation speed
> (time/LOC).

Sure, though that's the only thing we have. virtpatmat also introduced a
separate phase yet we didn't see such a degradation.
Using the current compiler to compile locker from pre-M3 will probably
give us the definitive conclusion on how good/bad we are compared to pre-M3.

>
> The current benchmarking results are close to useless when trying to
> infer performance. They're informative only with regards to CPU load
> on chara.
It runs on a separate machine afaik.

p.s. sorry for stealing the thread. I guess we can move this benchmark
discussion to a separate one if there is such a desire.

Eugene Burmako

unread,
Jun 4, 2012, 7:17:53 AM6/4/12
to scala-internals

Adriaan Moors

unread,
Jun 4, 2012, 7:20:51 AM6/4/12
to scala-i...@googlegroups.com
nice!

this is a good example of a self-contained commit of the right size
i'm not saying the whole reflection refactoring has to be split up like this (though that would be ideal),
but there must be more examples of changes like this that can be retained when moving from your current scratch pad to master

Josh Suereth

unread,
Jun 4, 2012, 9:45:53 AM6/4/12
to scala-i...@googlegroups.com

Yeah... is there anyway we can chunk up the refactoring more?

martin odersky

unread,
Jun 4, 2012, 10:17:36 AM6/4/12
to scala-i...@googlegroups.com
On Mon, Jun 4, 2012 at 3:45 PM, Josh Suereth <joshua....@gmail.com> wrote:

Yeah... is there anyway we can chunk up the refactoring more?

We'll need to look at it. Can't promise it yet. The main problem is getting stuff that builds and passes the tests.

Would it be better to have several, smaller intermediate pull requests that do not pass all the tests? 

If yes, that would simplify the task of breaking things out.

Cheers

 - Martin



--
Martin Odersky
Prof., EPFL and Chairman, Typesafe

Josh Suereth

unread,
Jun 4, 2012, 10:22:06 AM6/4/12
to scala-i...@googlegroups.com
Rock |  Reflection Pull Request | Hard Place


I'm pretty sure there's negatives to both sides of the coin here.  Maybe we should just pick which one we think is more useful overall and take that path?

I'm leaning towards trying to keep commits successfully building.

Eugene Burmako

unread,
Jun 4, 2012, 10:23:16 AM6/4/12
to scala-internals
There one more thing that needs to be resolved before the chunking.

If I factor out scala-reflect.jar, performance degrades by ~30s.

Without scala-reflect.jar locker takes: 1:32 + 2:12
With scala-reflect.jar it takes: 1:31 + 0:51 + 1:52

Some further experiments:
If I move all the files from scala/reflect to scala/library, then it
takes: 1:51 + 0:00 + 1:52
If I move a lot of files from scala/reflect to lib and comp, without
trivializing reflect [1], then it takes: 1:35 + 0:41 + 1:57

Any ideas what could be the reason for another performance problem?

[1] https://github.com/scalamacros/kepler/tree/d1c68b197bb4a60ad11ec4947e5e213440a12b5b

On Jun 4, 3:45 pm, Josh Suereth <joshua.suer...@gmail.com> wrote:
> Yeah... is there anyway we can chunk up the refactoring more?
> On Jun 4, 2012 7:21 AM, "Adriaan Moors" <adriaan.mo...@epfl.ch> wrote:
>
>
>
> > nice!
>
> > this is a good example of a self-contained commit of the right size
> > i'm not saying the whole reflection refactoring has to be split up like
> > this (though that would be ideal),
> > but there must be more examples of changes like this that can be retained
> > when moving from your current scratch pad to master
>
> > On Mon, Jun 4, 2012 at 1:17 PM, Eugene Burmako <eugene.burm...@epfl.ch>wrote:
>
> >>https://github.com/scalamacros/kepler/commit/6d20e13aa193d3e988ef3974...
>
> >> The regression is gone
>
> >> Running Jenkins right now:
> >>https://scala-webapps.epfl.ch/jenkins/job/scala-checkin-manual/57/con...

Adriaan Moors

unread,
Jun 4, 2012, 10:22:49 AM6/4/12
to scala-i...@googlegroups.com
As long as the compiler/library builds, I think it's preferable to have smaller commits even if they fail some tests
(please include the list of tests that are expected to fail in the commit message so that future archeologists
know what to expect)

Eugene Burmako

unread,
Jun 4, 2012, 10:26:44 AM6/4/12
to scala-i...@googlegroups.com
How many is "some"?

martin odersky

unread,
Jun 4, 2012, 10:32:04 AM6/4/12
to scala-i...@googlegroups.com
I think ~30s might be the time for a new compiler to get fully warmed up. I saw times for the 16K lines in package tools.nsc.typechecker go down from 48sec (cold) to 12sec (warm, library loaded).

Does ant start a new jvm for the reflect build?
That would explain it.

Or is it just a new classloader, or else a new global? In that case I do not have a good explanation.

Cheers

- Martin




--
Martin Odersky
Prof., EPFL and Chairman, Typesafe

Eugene Burmako

unread,
Jun 4, 2012, 10:32:25 AM6/4/12
to scala-i...@googlegroups.com
Well, okay, this can possibly be done, I think.

I'll need a day or two, though.

Adriaan Moors

unread,
Jun 4, 2012, 10:34:46 AM6/4/12
to scala-i...@googlegroups.com
On Mon, Jun 4, 2012 at 4:26 PM, Eugene Burmako <eugene....@epfl.ch> wrote:
How many is "some"?
It depends on the radix, but let's say <= 200

Let's try to steer clear from the rock and the hard place as much as possible.
This will require flexibility and creative maneuvering. That's fine.

The middle ground, to me, is something that can be done in a reasonable amount of time (I'd say a couple of man-days), 
and that allows us to diagnose the impact of these commits in the future.
Thus, the bar should be no lower than "all the code in this commit compiles" (excluding pos/ tests, but I hope it's clear enough what I'm saying).

Eugene Burmako

unread,
Jun 4, 2012, 10:35:17 AM6/4/12
to scala-i...@googlegroups.com
It creates a new <scalacfork />. Don't know how it works, though.

Josh Suereth

unread,
Jun 4, 2012, 10:37:40 AM6/4/12
to scala-i...@googlegroups.com
As the name suggests, scalafork forks.   Technically, we can stop using scalacfork, since all the features are in the scalac task, but for some reason that commit go reverted a few years back.

In any case, ANT is forking and does not cache scala compilers.   It would be ideal if this was an easy no-op to add, but honestly, I'd rather fix the SBT build for that.  It's much easier to code in Scala than XML.

- Josh

Eugene Burmako

unread,
Jun 4, 2012, 10:55:21 AM6/4/12
to scala-i...@googlegroups.com
It seems that I cant use <scala />, because it doesn't support some of the attributes that <scalacfork /> has.

Could you take a look at https://github.com/scala/scala/pull/663?

Mark Harrah

unread,
Jun 4, 2012, 11:08:12 AM6/4/12
to scala-i...@googlegroups.com
On Mon, 4 Jun 2012 10:37:40 -0400
Josh Suereth <joshua....@gmail.com> wrote:

> As the name suggests, scalafork forks. Technically, we can stop using
> scalacfork, since all the features are in the scalac task, but for some
> reason that commit go reverted a few years back.
>
> In any case, ANT is forking and does not cache scala compilers. It would
> be ideal if this was an easy no-op to add, but honestly, I'd rather fix the
> SBT build for that. It's much easier to code in Scala than XML.

As far as I know, the jvm couldn't handle all of the different versions of Scala being loaded and unloaded and ended up throwing OOME:PermGen. The previous attempt at the sbt build ran into this problem as well.

-Mark
> >> Prof., EPFL <http://www.epfl.ch> and Chairman, Typesafe<http://www.typesafe.com>

Josh Suereth

unread,
Jun 4, 2012, 1:04:37 PM6/4/12
to scala-i...@googlegroups.com
Yep, you pretty much have to fork here, especially in ANt.  Ant is no where near ready to deal with the complexities of not forking.

And as far as the curent SBT build, doesn't it stay in procss now?

Mark Harrah

unread,
Jun 4, 2012, 9:40:40 PM6/4/12
to scala-i...@googlegroups.com
On Mon, 4 Jun 2012 13:04:37 -0400
Josh Suereth <joshua....@gmail.com> wrote:

> Yep, you pretty much have to fork here, especially in ANt. Ant is no where
> near ready to deal with the complexities of not forking.
>
> And as far as the curent SBT build, doesn't it stay in procss now?

Yes. If you don't see memory issues, I don't know what has changed.

-Mark

Josh Suereth

unread,
Jun 4, 2012, 9:53:39 PM6/4/12
to scala-i...@googlegroups.com
caching, I thought...

Eugene Burmako

unread,
Jun 5, 2012, 8:14:26 PM6/5/12
to scala-internals, Adriaan Moors, Martin Odersky
Okay, this is what I've been able to factor out so far:

https://github.com/scalamacros/kepler/compare/db1b777372...84a254adbe

Some comments:
1) There's still the blob commit, though it's much less bloated than
before.
2) I think it'll be better to move test fixup into a separate commit,
or otherwise changes in tests completely overshadow everything else.
3) Some commits in the chain don't build yet - I'll address this
tomorrow.

Comments? Ideas? Suggestions?

On Jun 5, 3:53 am, Josh Suereth <joshua.suer...@gmail.com> wrote:
> caching, I thought...
>
>
>
> On Mon, Jun 4, 2012 at 9:40 PM, Mark Harrah <dmhar...@gmail.com> wrote:
> > On Mon, 4 Jun 2012 13:04:37 -0400
> > Josh Suereth <joshua.suer...@gmail.com> wrote:
>
> > > Yep, you pretty much have to fork here, especially in ANt.  Ant is no
> > where
> > > near ready to deal with the complexities of not forking.
>
> > > And as far as the curent SBT build, doesn't it stay in procss now?
>
> > Yes.  If you don't see memory issues, I don't know what has changed.
>
> > -Mark
>
> > > On Mon, Jun 4, 2012 at 11:08 AM, Mark Harrah <dmhar...@gmail.com> wrote:
>
> > > > On Mon, 4 Jun 2012 10:37:40 -0400
> > > > Josh Suereth <joshua.suer...@gmail.com> wrote:
>
> > > > > As the name suggests, scalafork forks.   Technically, we can stop
> > using
> > > > > scalacfork, since all the features are in the scalac task, but for
> > some
> > > > > reason that commit go reverted a few years back.
>
> > > > > In any case, ANT is forking and does not cache scala compilers.   It
> > > > would
> > > > > be ideal if this was an easy no-op to add, but honestly, I'd rather
> > fix
> > > > the
> > > > > SBT build for that.  It's much easier to code in Scala than XML.
>
> > > > As far as I know, the jvm couldn't handle all of the different
> > versions of
> > > > Scala being loaded and unloaded and ended up throwing OOME:PermGen.
> >  The
> > > > previous attempt at the sbt build ran into this problem as well.
>
> > > > -Mark
>
> > > > > - Josh
>
> > > > > On Mon, Jun 4, 2012 at 10:35 AM, Eugene Burmako <
> > eugene.burm...@epfl.ch
> > > > >wrote:
>
> > > > > > It creates a new <scalacfork />. Don't know how it works, though.
>
> > > > > > On 4 June 2012 16:32, martin odersky <martin.oder...@epfl.ch>
> > wrote:
>
> > > > > >> I think ~30s might be the time for a new compiler to get fully
> > warmed
> > > > up.
> > > > > >> I saw times for the 16K lines in package tools.nsc.typechecker go
> > > > down from
> > > > > >> 48sec (cold) to 12sec (warm, library loaded).
>
> > > > > >> Does ant start a new jvm for the reflect build?
> > > > > >> That would explain it.
>
> > > > > >> Or is it just a new classloader, or else a new global? In that
> > case I
> > > > do
> > > > > >> not have a good explanation.
>
> > > > > >> Cheers
>
> > > > > >> - Martin
>
> > > > > >> On Mon, Jun 4, 2012 at 4:23 PM, Eugene Burmako <
> > > > eugene.burm...@epfl.ch>wrote:
>
> > > > > >>> There one more thing that needs to be resolved before the
> > chunking.
>
> > > > > >>> If I factor out scala-reflect.jar, performance degrades by ~30s.
>
> > > > > >>> Without scala-reflect.jar locker takes: 1:32 + 2:12
> > > > > >>> With scala-reflect.jar it takes: 1:31 + 0:51 + 1:52
>
> > > > > >>> Some further experiments:
> > > > > >>> If I move all the files from scala/reflect to scala/library,
> > then it
> > > > > >>> takes: 1:51 + 0:00 + 1:52
> > > > > >>> If I move a lot of files from scala/reflect to lib and comp,
> > without
> > > > > >>> trivializing reflect [1], then it takes: 1:35 + 0:41 + 1:57
>
> > > > > >>> Any ideas what could be the reason for another performance
> > problem?
>
> > > > > >>> [1]
>
> >https://github.com/scalamacros/kepler/tree/d1c68b197bb4a60ad11ec4947e...

Adriaan Moors

unread,
Jun 6, 2012, 5:45:20 AM6/6/12
to Eugene Burmako, scala-internals, Martin Odersky
I think you're striking the right balance in the commits you've already split up. Good work!
I'll hold off reviewing until The Blob has been pared down.

Eugene Burmako

unread,
Jun 6, 2012, 5:53:54 AM6/6/12
to scala-internals
Yay, the blob is under 200 files now!

But I'm reaching the limit here, and changed LOC is still very high.
That's natural, because, for example, we killed a dummy (almost 1kloc)
and made trees abstract (+1.5 kloc). Also, adding a new layer to the
compiler super-cake also isn't loc-free.

Now I only have to make it compile and to propagate all the changes I
made to the blob up and down the commit chain. As the number of
commits grows, refactorings become much harder :(

On Jun 6, 11:45 am, Adriaan Moors <adriaan.mo...@epfl.ch> wrote:
> I think you're striking the right balance in the commits you've already
> split up. Good work!
> I'll hold off reviewing until The Blob has been pared down.
>

Eugene Burmako

unread,
Jun 6, 2012, 6:03:00 AM6/6/12
to scala-internals
Here's the breakdown based on
https://github.com/scalamacros/kepler/commit/8f695fc07e89f00f673f592118c978309ecda70f:

Showing 196 changed files with 9,261 additions and 6,531 deletions.

Definitions 0.5 kloc
Names 0.3 kloc
Dummy 0.8 kloc
Toolboxes 0.4 kloc
Mirrors 2.4 kloc
Trees: 3.1 kloc
Base: 2.3 kloc (not counting the trees)
====
These guys contribute to almost 10kloc changed of the entire 15kloc.

Eugene Burmako

unread,
Jun 6, 2012, 11:09:13 AM6/6/12
to scala-internals, Adriaan Moors
Allrighty, I'm done. Here's the new wannabe pull request:

https://github.com/scalamacros/kepler/commits/pullrequest/reflection

As soon as Jenkins approves it, I'll submit it to trunk.

On Jun 6, 12:03 pm, Eugene Burmako <eugene.burm...@epfl.ch> wrote:
> Here's the breakdown based onhttps://github.com/scalamacros/kepler/commit/8f695fc07e89f00f673f5921...

Eugene Burmako

unread,
Jun 6, 2012, 12:29:23 PM6/6/12
to scala-internals
BUILD SUCCESSFUL
+ exit 0
Finished: SUCCESS

I'll wait with submitting a pull request, though, because of reported
problems with -sourcepath and source compatibility.

The former is handled by Vlad, the latter we'll discuss tomorrow
during reflection meeting.

iulian dragos

unread,
Jun 6, 2012, 12:50:13 PM6/6/12
to scala-i...@googlegroups.com
On Wed, Jun 6, 2012 at 6:29 PM, Eugene Burmako <eugene....@epfl.ch> wrote:
> BUILD SUCCESSFUL
> + exit 0
> Finished: SUCCESS
>
> I'll wait with submitting a pull request, though, because of reported
> problems with -sourcepath and source compatibility.

It doesn't make much of a difference if master has the changes or not,
as long as the nightly is your branch. In fact, I'd slightly favor a
reconciliation between published nightlies and the canonical
repository :)

What really matters is to fix these issues quickly, so we don't lose
another day. At least the sourcepath :)

cheers,
iulian
--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

Eugene Burmako

unread,
Jun 7, 2012, 10:03:23 AM6/7/12
to scala-internals, Martin Odersky
By the way, folks, with the scala-reflect.jar move we're going to lose
history of directories.

From a discussion in a nearby thread, that can't be helped,
unfortunately: http://groups.google.com/group/scala-internals/browse_thread/thread/7a34255fda6c0580.
Reply all
Reply to author
Forward
0 new messages