Optimize QTree a bunch #475
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
| } | ||
|
|
||
| class QTreeSemigroup[A](k: Int)(implicit val underlyingMonoid: Monoid[A]) extends Semigroup[QTree[A]] { | ||
| class QTreeSemigroup[@specialized(Int, Long, Float, Double, Unit) A](k: Int)(implicit val underlyingMonoid: Monoid[A]) extends Semigroup[QTree[A]] { |
There was a problem hiding this comment.
do we need the Unit? There is only one value, so I assume boxing is fast. If we specialize on it, we get an additional copy of all the methods with type parameters, which adds to the jar size.
There was a problem hiding this comment.
Probably not, will remove
|
A+++ would write unoptimized code for ianoc to make much better again. |
ianoc
force-pushed
the
ianoc/qTreeBenchmarkMoreCoverage
branch
from
3bb335a to
fd9f4db
Compare
Heavily driven by benchmarks. Avoid allocations, dealing with options or similar. Attempt to keep as much external source compatibility as possible
ianoc
force-pushed
the
ianoc/optimizeQTree
branch
from
ec320e3 to
e8f869b
Compare
…t erased than the old constructor. Add the old constructor as an overloaded constructor so that we can keep our binary compatibility to a minimum
We now are just missing copy constructors:
[error] * synthetic method copy$default$6()scala.Option in class com.twitter.algebird.QTree does not have a correspondent in new version
[error] filter with: ProblemFilters.exclude[MissingMethodProblem]("com.twitter.algebird.QTree.copy$default$6")
[error] * synthetic method com$twitter$algebird$QTree$$findRankUpperBound(Long)scala.Option in class com.twitter.algebird.QTree does not have a correspondent in new version
[error] filter with: ProblemFilters.exclude[MissingMethodProblem]("com.twitter.algebird.QTree.com$twitter$algebird$QTree$$findRankUpperBound")
[error] * synthetic method copy$default$2()Int in class com.twitter.algebird.QTree does not have a correspondent in new version
[error] filter with: ProblemFilters.exclude[MissingMethodProblem]("com.twitter.algebird.QTree.copy$default$2")
[error] * synthetic method copy$default$5()scala.Option in class com.twitter.algebird.QTree does not have a correspondent in new version
[error] filter with: ProblemFilters.exclude[MissingMethodProblem]("com.twitter.algebird.QTree.copy$default$5")
[error] * synthetic method com$twitter$algebird$QTree$$findRankLowerBound(Long)scala.Option in class com.twitter.algebird.QTree does not have a correspondent in new version
[error] filter with: ProblemFilters.exclude[MissingMethodProblem]("com.twitter.algebird.QTree.com$twitter$algebird$QTree$$findRankLowerBound")
[error] * method copy(Long,Int,Long,java.lang.Object,scala.Option,scala.Option)com.twitter.algebird.QTree in class com.twitter.algebird.QTree does not have a correspondent in new version
[error] filter with: ProblemFilters.exclude[MissingMethodProblem]("com.twitter.algebird.QTree.copy")
[error] * synthetic method copy$default$1()Long in class com.twitter.algebird.QTree does not have a correspondent in new version
[error] filter with: ProblemFilters.exclude[MissingMethodProblem]("com.twitter.algebird.QTree.copy$default$1")
[error] * synthetic method copy$default$4()java.lang.Object in class com.twitter.algebird.QTree does not have a correspondent in new version
[error] filter with: ProblemFilters.exclude[MissingMethodProblem]("com.twitter.algebird.QTree.copy$default$4")
[error] * synthetic method copy$default$3()Long in class com.twitter.algebird.QTree does not have a correspondent in new version
[error] filter with: ProblemFilters.exclude[MissingMethodProblem]("com.twitter.algebird.QTree.copy$default$3")
java.lang.RuntimeException: algebird-core: Binary compatibility check failed!
Do we mind that?
| @@ -155,7 +243,7 @@ case class QTree[A]( | |||
| ancestorLevel += 1 | |||
| offsetDiff >>= 1 | |||
| } | |||
| ancestorLevel.max(level).max(other.level) | |||
| ancestorLevel.max(_level).max(other.level) | |||
There was a problem hiding this comment.
Maybe use scala.math.max here to prevent boxing/unboxing?
There was a problem hiding this comment.
I don't think these box. I think they use AnyVal enrichments:
|
@avibryant Any time! I never have the time to play with new algo's. But once our jobs go slow lots of motivation to improve things! :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Performance numbers:
Develop::
[info] Benchmark (depthK) (numElements) Mode Cnt Score Error Units
[info] QTreeBenchmark.timePlusDouble 6 10000 avgt 15 49604065.294 ± 343946.724 ns/op
[info] QTreeBenchmark.timePlusLong 6 10000 avgt 15 54040361.060 ± 451564.394 ns/op
[info] QTreeBenchmark.timePlusUnit 6 10000 avgt 15 52363803.668 ± 1403481.464 ns/op
[info] QTreeBenchmark.timeSumOptionDouble 6 10000 avgt 15 18536550.306 ± 102450.054 ns/op
[info] QTreeBenchmark.timeSumOptionLong 6 10000 avgt 15 14265313.429 ± 80999.618 ns/op
[info] QTreeBenchmark.timeSumOptionUnit 6 10000 avgt 15 14109388.526 ± 139901.693 ns/op
Branch::
[info] Benchmark (depthK) (numElements) Mode Cnt Score Error Units
[info] QTreeBenchmark.timePlusDouble 6 10000 avgt 15 36130358.089 ± 344895.234 ns/op
[info] QTreeBenchmark.timePlusLong 6 10000 avgt 15 40619584.216 ± 213205.614 ns/op
[info] QTreeBenchmark.timePlusUnit 6 10000 avgt 15 35111406.421 ± 175324.238 ns/op
[info] QTreeBenchmark.timeSumOptionDouble 6 10000 avgt 15 8657591.363 ± 148336.269 ns/op
[info] QTreeBenchmark.timeSumOptionLong 6 10000 avgt 15 5310313.465 ± 88287.513 ns/op
[info] QTreeBenchmark.timeSumOptionUnit 6 10000 avgt 15 4735604.540 ± 45094.848 ns/op