Skip to content

Conversation

@RichardBradley
Copy link
Contributor

This change is to scan the measurement files line-by-line, rather than loading the whole thing into memory as a giant string.

With a large test suite, the measurement files can get rather large (30MB for half of our suite).

If we change the "invoked" numbers to be \n delimited instead of ; delimited, then we can use the standard line-by-line buffered file reading libs to read in the statement ids instead of needing to read a huge string then doing a (memory-expensive) split into a huge array.

Note that this creates a binary incompatibility -- you will need to recompile sbt-scoverage (no source changes are needed in that project).

This takes my test run (sbt scoverage:test, without a "clean") from 1m28 to 1m22. This number may overstate the improvement (or hopefully understate it); I haven't done very many tests.

Performance measurements digression

Just for interest, while I'm measuring performance, here are some numbers from our fairly large multi-threaded test suite (477 tests). Note that this is just the "test" part, not the integration test part. This is on my Windows dev machine, using Java 6 and Scala 2.10.3

Time to run "sbt scoverage:test" without a clean (i.e. after a previous run has done all the compilation):

  • At revision 2acb521 = 10m52
  • After today's 4 pull requests = 1m22

I don't quite believe this, but I have just checked and re-checked this, and it seems like "sbt test" takes longer than "sbt scoverage:test", at about 2m06. I'm going to have to dig into that!

(The variance of each of the measured numbers above looks to be around 5%)

@sksamuel
Copy link
Member

Looks good. The performance improvements are excellent.
Btw you don't need to rebuild the sbt plugin if you change the scala plugin, as its not "included" in the sbt plugin. Obviously you need to build sbt plugin if you change the version number it refers to.

@RichardBradley
Copy link
Contributor Author

Btw you don't need to rebuild the sbt plugin if you change the scala plugin, as its not "included" in the sbt plugin

In general, that's right. You have to rebuild it after this change though, due to the change in the signature of "invoked"

@RichardBradley
Copy link
Contributor Author

I forgot to update the tests; I'll fix (EDIT: now done)

@sksamuel
Copy link
Member

Oh I see you changed it to a Set not a Seq.

@RichardBradley
Copy link
Contributor Author

I have just checked and re-checked this, and it seems like "sbt test" takes longer than "sbt scoverage:test"

This is still the case for me: uncovered test reproducibly takes 2m48 +/- 5 and scovered test reproducibly takes 1m21 +/- 2.

I have checked "inspect test" and "inspect scoverage:test", "inspect scoverage-test:test" in SBT and I can't see any extra work being done in test. AFAICT "scoverage:test" should do exactly the same stuff as test, except using a different classpath and with the extra "cleanup" step to generate the report.

I have checked the detailed output: each individual test is faster uncovered, but the whole thing is slower. Something very funny is going on. It's probably not scoverage's fault: I reckon I have misconfigured SBT or something.

@sksamuel
Copy link
Member

Are you sure that scoverage is running all the tests ok and not skipping
any?

On 15 May 2014 20:29, Richard Bradley notifications@github.com wrote:

I have just checked and re-checked this, and it seems like "sbt test"
takes longer than "sbt scoverage:test"

This is still the case for me: uncovered test reproducibly takes 2m48 +/-
5 and scovered test reproducibly takes 1m21 +/- 2.

I have checked "inspect test" and "inspect scoverage:test", "inspect
scoverage-test:test" in SBT and I can't see any extra work being done in
test. AFAICT "scoverage:test" should do exactly the same stuff as test,
except using a different classpath and with the extra "cleanup" step to
generate the report.

I have checked the detailed output: each individual test is faster
uncovered, but the whole thing is slower. Something very funny is going on.
It's probably not scoverage's fault: I reckon I have misconfigured SBT or
something.


Reply to this email directly or view it on GitHubhttps://github.com//pull/38#issuecomment-43254325
.

sksamuel added a commit that referenced this pull request May 15, 2014
Don't load the whole measurement file into memory at once -- it may be very large
@sksamuel sksamuel merged commit 35fe308 into scoverage:master May 15, 2014
@RichardBradley
Copy link
Contributor Author

Found it -- tests are not being run in parallel in "Test".

I tried to turn that on for our integration tests, but it looks like it's serial execution in Test and Integration, but parallel execution in scoverage-test.

@sksamuel
Copy link
Member

That make sense.

On 15 May 2014 21:13, Richard Bradley notifications@github.com wrote:

Found it -- tests are not being run in parallel in "Test".

I tried to turn that on for our integration tests, but it looks like it's
serial execution in Test and Integration, but parallel execution in
scoverage-test.


Reply to this email directly or view it on GitHubhttps://github.com//pull/38#issuecomment-43259057
.

@RichardBradley RichardBradley deleted the report-memory-perf branch June 9, 2014 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants