[ci: last-only] merge 2.11.x onto 2.12.x (February 14, 2017) #5701
Conversation
This file contains hidden or 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
While investigating scala/scala-dev#251
While investigating scala/scala-dev#251
…ckport] Hash consing of trees within pattern match analysis was broken, and considered `x1.foo#1` to be the same tree as `x1.foo#2`, even though the two `foo`-s referred to different symbols. The hash consing was based on `Tree#correspondsStructure`, but the predicate in that function cannot veto correspondance, it can only supplement the default structural comparison. I've instead created a custom tree comparison method for use in the pattern matcher that handles the tree shapes that we use. (cherry picked from commit 79a52e6)
As they're reusable in 2.12.x with this change[1], it'd be useful to make them reusable in 2.11.x. [1] scala@6eaae1b With this change, not only are they reusable but also we can avoid mutation of previously created arrays. Behaviour(Problem): Actual behaviour before this modification is as follows; <ArrayBuilder> ``` scala> import scala.collection.mutable.ArrayBuilder import scala.collection.mutable.ArrayBuilder scala> val builder = new ArrayBuilder.ofInt builder: scala.collection.mutable.ArrayBuilder.ofInt = ArrayBuilder.ofInt scala> builder ++= Vector.range(1, 17) res0: builder.type = ArrayBuilder.ofInt scala> val arr = builder.result() arr: Array[Int] = Array(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) scala> builder.clear() scala> builder += 100 res2: builder.type = ArrayBuilder.ofInt scala> val arr2 = builder.result() arr2: Array[Int] = Array(100) scala> arr res3: Array[Int] = Array(100, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) // arr should be Array(1, .., 16) but was unexpectedly mutated by `+=` operation ``` `arr` was mutated as follows; 1. `result` & `clear` - `arr = elems` - `size = 0` 2. `+=(100)` - `ensureSize(0 + 1)` => `capacity < size || capacity == 0` is `false` as `capacity == 16` and `size == 1` - `elems(0) = 100` this is wh 10000 ere `arr(0) = 100` was done because we did not reallocate a new array for `elems` when calling `ensureSize`, which should have happened. - `size = 1` 3. `result` - `mkArray(1)` gives us `arr2 = Array(100)` <WrappedArrayBuilder> We can observe almost the same mutation behaviour of ArrayBuilder. ``` scala> import scala.collection.mutable.WrappedArray import scala.collection.mutable.WrappedArray scala> import scala.collection.mutable.WrappedArrayBuilder import scala.collection.mutable.WrappedArrayBuilder scala> import scala.reflect.ClassTag import scala.reflect.ClassTag scala> val builder = new WrappedArrayBuilder(ClassTag.Int) builder: scala.collection.mutable.WrappedArrayBuilder[Int] = scala.collection.mutable.WrappedArrayBuilder@56cbfb61 scala> builder ++= Vector.range(1, 17) res0: builder.type = scala.collection.mutable.WrappedArrayBuilder@56cbfb61 scala> builder.result() res1: scala.collection.mutable.WrappedArray[Int] = WrappedArray(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) scala> builder.clear() scala> builder += 100 res3: builder.type = scala.collection.mutable.WrappedArrayBuilder@56cbfb61 scala> res1 res4: scala.collection.mutable.WrappedArray[Int] = WrappedArray(100, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16) scala> builder.result() res5: scala.collection.mutable.WrappedArray[Int] = WrappedArray(100) ``` Solution: We should reset `capacity` to `0` when calling `result` so that `ensureSize(1)` calls `resize(16)`, which satisfies the property of Builder reusability. Besides mutation of previously created arrays does not happen.
`enqueue` appends elements to the `Queue`, it doesn't prepend them.
Fix documentation of immutable.Queue
Cleanup in aisle patmat
(cherry picked from commit 9a24860)
(`ops/s`, smaller is better) `Before (9c5d3f8)`: ```scala [info] # Run complete. Total time: 00:08:15 [info] [info] Benchmark (size) Mode Cnt Score Error Units [info] s.c.immutable.VectorMapBenchmark.groupBy 10 avgt 20 645.594 ± 9.435 ns/op [info] s.c.immutable.VectorMapBenchmark.groupBy 100 avgt 20 2084.216 ± 37.814 ns/op [info] s.c.immutable.VectorMapBenchmark.groupBy 1000 avgt 20 19878.481 ± 262.404 ns/op [info] s.c.mutable.HashMapBenchmark.get 10 avgt 20 689.941 ± 5.850 ns/op [info] s.c.mutable.HashMapBenchmark.get 100 avgt 20 7357.330 ± 45.956 ns/op [info] s.c.mutable.HashMapBenchmark.get 1000 avgt 20 95767.200 ± 1550.771 ns/op [info] s.c.mutable.HashMapBenchmark.getOrElseUpdate 10 avgt 20 509.181 ± 2.683 ns/op [info] s.c.mutable.HashMapBenchmark.getOrElseUpdate 100 avgt 20 5563.301 ± 32.335 ns/op [info] s.c.mutable.HashMapBenchmark.getOrElseUpdate 1000 avgt 20 71965.365 ± 1809.738 ns/op [info] s.c.mutable.HashMapBenchmark.put 10 avgt 20 247.270 ± 3.972 ns/op [info] s.c.mutable.HashMapBenchmark.put 100 avgt 20 5646.185 ± 106.172 ns/op [info] s.c.mutable.HashMapBenchmark.put 1000 avgt 20 81303.663 ± 954.938 ns/op ``` `Changed modulo to bitwise and in hash calculation (4c729fe)`: ```scala [info] Benchmark (size) Mode Cnt Score Error Units [info] s.c.immutable.VectorMapBenchmark.groupBy 10 avgt 20 631.291 ± 9.269 ns/op [info] s.c.immutable.VectorMapBenchmark.groupBy 100 avgt 20 2077.885 ± 59.737 ns/op [info] s.c.immutable.VectorMapBenchmark.groupBy 1000 avgt 20 15458.278 ± 317.347 ns/op [info] s.c.mutable.HashMapBenchmark.get 10 avgt 20 678.013 ± 4.453 ns/op [info] s.c.mutable.HashMapBenchmark.get 100 avgt 20 7258.522 ± 76.088 ns/op [info] s.c.mutable.HashMapBenchmark.get 1000 avgt 20 94748.845 ± 1226.120 ns/op [info] s.c.mutable.HashMapBenchmark.getOrElseUpdate 10 avgt 20 498.042 ± 5.006 ns/op [info] s.c.mutable.HashMapBenchmark.getOrElseUpdate 100 avgt 20 5243.154 ± 110.372 ns/op [info] s.c.mutable.HashMapBenchmark.getOrElseUpdate 1000 avgt 20 68194.752 ± 655.436 ns/op [info] s.c.mutable.HashMapBenchmark.put 10 avgt 20 257.275 ± 1.411 ns/op [info] s.c.mutable.HashMapBenchmark.put 100 avgt 20 5318.532 ± 152.923 ns/op [info] s.c.mutable.HashMapBenchmark.put 1000 avgt 20 79607.160 ± 651.779 ns/op ``` `Optimized HashTable.index (6cc1504)`: ```scala [info] Benchmark (size) Mode Cnt Score Error Units [info] s.c.immutable.VectorMapBenchmark.groupBy 10 avgt 20 616.164 ± 4.712 ns/op [info] s.c.immutable.VectorMapBenchmark.groupBy 100 avgt 20 2034.447 ± 14.495 ns/op [info] s.c.immutable.VectorMapBenchmark.groupBy 1000 avgt 20 14712.164 ± 119.983 ns/op [info] s.c.mutable.HashMapBenchmark.get 10 avgt 20 679.046 ± 6.872 ns/op [info] s.c.mutable.HashMapBenchmark.get 100 avgt 20 7242.097 ± 41.244 ns/op [info] s.c.mutable.HashMapBenchmark.get 1000 avgt 20 95342.919 ± 1521.328 ns/op [info] s.c.mutable.HashMapBenchmark.getOrElseUpdate 10 avgt 20 488.034 ± 4.554 ns/op [info] s.c.mutable.HashMapBenchmark.getOrElseUpdate 100 avgt 20 4883.123 ± 59.268 ns/op [info] s.c.mutable.HashMapBenchmark.getOrElseUpdate 1000 avgt 20 65174.034 ± 496.759 ns/op [info] s.c.mutable.HashMapBenchmark.put 10 avgt 20 267.983 ± 1.797 ns/op [info] s.c.mutable.HashMapBenchmark.put 100 avgt 20 5097.351 ± 104.538 ns/op [info] s.c.mutable.HashMapBenchmark.put 1000 avgt 20 78772.540 ± 543.935 ns/op ``` Summary, i.e. the effect of this PR, according to the benchmarks: * `groupBy` has a `~35%` speedup * `get` didn't change * `getOrElseUpdate` has a `~10%` speedup * `put` has a `~3%` speedup Note: caching the `exponent` to a local private field (`Byte` or `Int`) didn't have any performance advantage (only a minor slowdown was measured, possibly because it's accessed via an interface now) (cherry picked from commit a501444)
(cherry picked from commit 7952525)
(cherry picked from commit bc91223)
Fixes https://issues.scala-lang.org/browse/SI-10049 Since `groupBy` uses this method extensively and suffered a measurable slowdown in `2.12.0`, this modification restores (and exceeds) its original speed. --- included benchmarks: (`ns/op` → smaller is better) `before (2.12.0):` ```java Benchmark (size) Mode Cnt Score Error Units s.c.immutable.VectorMapBenchmark.groupBy 10 avgt 20 865.693 ± 7.869 ns/op s.c.immutable.VectorMapBenchmark.groupBy 100 avgt 20 3095.657 ± 56.438 ns/op s.c.immutable.VectorMapBenchmark.groupBy 1000 avgt 20 28247.005 ± 470.513 ns/op s.c.mutable.HashMapBenchmark.get 10 avgt 20 679.448 ± 11.809 ns/op s.c.mutable.HashMapBenchmark.get 100 avgt 20 7240.178 ± 61.734 ns/op s.c.mutable.HashMapBenchmark.get 1000 avgt 20 95725.127 ± 2373.458 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 10 avgt 20 836.561 ± 20.085 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 100 avgt 20 7891.368 ± 56.808 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 1000 avgt 20 97478.629 ± 1782.497 ns/op s.c.mutable.HashMapBenchmark.put 10 avgt 20 243.422 ± 2.915 ns/op s.c.mutable.HashMapBenchmark.put 100 avgt 20 5810.927 ± 60.054 ns/op s.c.mutable.HashMapBenchmark.put 1000 avgt 20 82175.539 ± 1690.296 ns/op ``` `after:` ```java Benchmark (size) Mode Cnt Score Error Units s.c.immutable.VectorMapBenchmark.groupBy 10 avgt 20 627.007 ± 9.718 ns/op s.c.immutable.VectorMapBenchmark.groupBy 100 avgt 20 2086.955 ± 19.042 ns/op s.c.immutable.VectorMapBenchmark.groupBy 1000 avgt 20 19515.234 ± 173.647 ns/op s.c.mutable.HashMapBenchmark.get 10 avgt 20 683.977 ± 11.843 ns/op s.c.mutable.HashMapBenchmark.get 100 avgt 20 7345.675 ± 41.092 ns/op s.c.mutable.HashMapBenchmark.get 1000 avgt 20 95085.926 ± 1702.997 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 10 avgt 20 503.208 ± 2.643 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 100 avgt 20 5526.483 ± 28.262 ns/op s.c.mutable.HashMapBenchmark.getOrElseUpdate 1000 avgt 20 69265.900 ± 674.958 ns/op s.c.mutable.HashMapBenchmark.put 10 avgt 20 252.481 ± 7.597 ns/op s.c.mutable.HashMapBenchmark.put 100 avgt 20 5708.034 ± 110.360 ns/op s.c.mutable.HashMapBenchmark.put 1000 avgt 20 82051.378 ± 1432.009 ns/op ``` i.e. for the given benchmark conditions `~40%` faster `groupBy` and `getOrElseUpdate` (cherry picked from commit b67ca7d)
(cherry picked from commit 26c87f1)
[backport] Hash table getOrElseUpdate and indexing
…sability-bug-2016-12-24 [Backport] Modify ArrayBuilder and WrappedArrayBuilder to be reusable
When generating a varargs forwarder for
def foo[T](a: T*)
the parameter type of the forwarder needs to be Array[Object]. If we
generate Array[T] in UnCurry, that would be erased to plain Object, and
the method would not be a valid varargs.
Unfortunately, setting the parameter type to Array[Object] lead to
an invalid generic signature - the generic signature should reflect the
real signature.
This change adds an attachment to the parameter symbol in the varargs
forwarder method and special-cases signature generation.
Also cleans up the code to produce the varargs forwarder. For example,
type parameter and parameter symbols in the forwarder's method type were
not clones, but the same symbols from the original method were re-used.
Backported from 0d2760d,
with a small tweak (checkVarargs) to make the test work on Java 6,
as well as later versions.
Make sure that methods annotated with varargs are properly mixed-in. This commit splits the transformation into an info transformer (that works on all symbols, whether they come from source or binary) and a tree transformer. The gist of this is that the symbol-creation part of the code was moved to the UnCurry info transformer, while tree operations remained in the tree transformer. The newly created symbol is attached to the original method so that the tree transformer can still retrieve the symbol. A few fall outs: - I removed a local map that was identical to TypeParamsVarargsAttachment - moved the said attachment to StdAttachments so it’s visible between reflect.internal and nsc.transform - a couple more comments in UnCurry to honour the boy-scout rule
Cloning the original symbol in its entirety, rather than cloning its type/value parameters individually. `cloneSymbol` takes care of all the tricky substitutions for us!
Time for the courage of our convictions: follow the advice of my TODO comment from SI-8244 / f62e280 and fix `classExistentialType` once and for all. This is the change in the generated `canEquals` method in the test case; we no longer get a kind conformance error. ``` --- sandbox/old.log 2015-05-27 14:31:27.000000000 +1000 +++ sandbox/new.log 2015-05-27 14:31:29.000000000 +1000 @@ -15,7 +15,7 @@ case _ => throw new IndexOutOfBoundsException(x$1.toString()) }; override <synthetic> def productIterator: Iterator[Any] = runtime.this.ScalaRunTime.typedProductIterator[Any](Stuff.this); - <synthetic> def canEqual(x$1: Any): Boolean = x$1.$isInstanceOf[Stuff[Proxy[PP]]](); + <synthetic> def canEqual(x$1: Any): Boolean = x$1.$isInstanceOf[Stuff[_ <: [PP]Proxy[PP]]](); override <synthetic> def hashCode(): Int = ScalaRunTime.this._hashCode(Stuff.this); override <synthetic> def toString(): String = ScalaRunTime.this._toString(Stuff.this); override <synthetic> def equals(x$1: Any): Boolean = x$1 match { @@ -38,9 +38,3 @@ } } ``` I also heeded my own advice to pass in a prefix to this method.
When determining whether or not a pattern match requires an equality check of the outer instance of a type in addition to a type test, `needsOuterTest` determines if the intersection of the selector and the pattern types could be populated. Both type arrive at `isPopulated` dealised. However, `isPopulated` recurs when it encounters an existential, and, as seen in thest case, a failure to dealias the quantified type can lead to an assertion failure as we try to relate a typeref to an alias and a typeref to a class. See also SI-7214, which added deliasing of the pattern type before calling `isPopulated`.
Cherry-picked 365ac03 (and bumped from 2.14.1 to 2.14.3) Motivated by the improvements to multi-byte character handling. Screencast showing the reported bug is fixed: http://g.recordit.co/ie1Z367NUl.gif Here's the changelog since JLine 2.12.1: jline/jline2@jline-2.12.1...jline-2.14.3 I needed to disable a new, on-by-default feature in JLine so that it didn't add a " " after completing the token `equals` in `foo.equa<TAB>`.
Now that we use a release of JLine that includes the fix for: jline/jline2#208 We no longer need to the workaround introduced in 7719a3c. Screencast of the still-fixed behaviour: http://recordit.co/5pzh9OhlQv.gif (cherry picked from commit 6871bcc)
(cherry-picking commit a03e7a0) I have repeatedly seen this fail CI runs, including recently as the comment in the test itself says: "I'm not sure this is a great way to test for memory leaks, since we're also testing how good the JVM's GC is, and this is not easily reproduced between machines/over time"
Upgrade to jline2 2.14.3
SI-9331 Fix canEqual for case classes with HK type params
SI-9114 Fix crasher in pattern matcher with type aliases
[backport] SI-10071 SI-8786 varargs methods
We introduce a package-private superclass with the overridden implementations. This is public at the bytecode level, so it needs to be whitelisted.
Optimise common operations on Array and List
follows up on scala#5664; fixes scala/scala-dev#301
…-ant fix IndexedSeqTest to work in both Ant and sbt
|
this is WIP. let's see what Jenkins says about tests and about MiMa. the MiMa piece of this is not done yet. 5664 produced nasty merge conflicts in the whitelist files, which I plan to deal with by just seeing what Jenkins says and using that as a guide for what I need to add to the whitelist files. |
7054d55 to
d66d843
Compare
bah, it's an interface, so we can't do that. http://stackoverflow.com/questions/37829160/safevarargs-on-interface-method#37959022 |
d66d843 to
e6018d3
Compare
e6018d3 to
04c45e1
Compare
|
Jenkins is rushing from PR to PR like the protagonist of William Pène du Bois's “The Twenty-One Balloons” on the balloon platform. hang in there, Jenkins. |
adriaanm
approved these changes
Feb 17, 2017
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.
many of the PRs merged for 2.11.9 were backports, so we proceed
by considering each PR individually.
git log --merges origin/2.11.x ^origin/2.12.xshows:5612 is a special case: part backport, part not. I consulted with
Adriaan and he said to handle 3 commits as follows:
to avoid extra merge commits, we don't need to merge every PR individually,
we can chunk them according to whether they are backports or not. so that yields: