Skip to content
This repository was archived by the owner on Jun 10, 2020. It is now read-only.

Performance optimizations around logging#152

Merged
eed3si9n merged 2 commits intosbt:1.1.xfrom
retronym:topic/format
Mar 1, 2018
Merged

Performance optimizations around logging#152
eed3si9n merged 2 commits intosbt:1.1.xfrom
retronym:topic/format

Conversation

@retronym
Copy link
Member

@retronym retronym commented Feb 17, 2018

  • Cache the TypeTag[SuccessEvent] used in the frequently called ManagedLogger.success. The TypeTag itself has a lazy val tpe field that caches the Type, so we save some work in creating calling tpe.toString later on.
  • Reuse the StringBuilder used to buffer console output, rather than creating new one for each line.
  • Use StringBuilder rather than string interpolation in some hot code. A future version of Scala should make the interpolators just as fast, but right now there is some overhead.

@eed3si9n
Copy link
Member

@typesafe-tools
Copy link

The validator has checked the following projects against Scala 2.12,
tested using dbuild, projects built on top of each other.

Project Reference Commit
sbt 1.1.x sbt/sbt@7dcb311
zinc 1.1.x sbt/zinc@23186cd
io 1.1.x sbt/io@13eb241
librarymanagement 1.1.x sbt/librarymanagement@69fb352
util pull/152/head b68071a
website 1.1.x

✅ The result is: SUCCESS
(restart)

@retronym retronym changed the title WIP Cache evidence params for hot method Performance optimizations around logging Feb 23, 2018
@retronym retronym requested a review from eed3si9n February 23, 2018 04:57
@retronym
Copy link
Member Author

@eed3si9n Cleaned up and ready for review.

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dwijnand
Copy link
Member

can we revert the string building changes back to the interpolators in 2.12.5, @retronym, given scala/scala#6093?

Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall LGTM
Do you have some numbers we can quote?

@retronym
Copy link
Member Author

retronym commented Mar 1, 2018

No, I'm afraid I don't. I noticed differences in the profiles before and after, but didn't have a good way to measure the reduction in CPU time.

I was hoping to collect better numbers for this change (and future ones) by rigging up SBT under JMH using a custom profiler like in: scala/compiler-benchmark#61, but I've been unable to run SBT programattically (as opposed to through its launcher).

@retronym
Copy link
Member Author

retronym commented Mar 1, 2018

@dwijnand Yep, hopefully we'll get the opts into the built-in interpolator. That said, this sort of low level infrastructure code should be written for a wide variety of workloads, which means it is wise to avoid too many temporary objects, so even factoring out methods that are internally efficient with the new s but just create strings that are immediately concatenated is a bad idea.

@eed3si9n eed3si9n merged commit 226a543 into sbt:1.1.x Mar 1, 2018
@dwijnand
Copy link
Member

dwijnand commented Mar 1, 2018

I've been unable to run SBT programattically (as opposed to through its launcher)

RunFromSourceMain is my (ongoing) attempt to finding a way to launch sbt without going through the launcher.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants