Skip to content

Commit 0e29386

Browse files
[compiler] get rid of BoxedArrayBuilder (#15131)
## Change Description This PR makes a start at replacing Hail's old custom collection classes with Scala 2.13 style collections. Here, I just replace all uses of `BoxedArrayBuilder` with alternatives using the standard library. For most cases, I either replace it with an `ArrayBuffer` if access to the contents is needed during construction, and an `ArraySeq.newBuilder` otherwise. ## Security Assessment - This change cannot impact the Hail Batch instance as deployed by Broad Institute in GCP
1 parent 0f616c6 commit 0e29386

File tree

81 files changed

+532
-585
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

81 files changed

+532
-585
lines changed

hail/build.mill

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ trait HailModule extends ScalaModule with ScalafmtModule with ScalafixModule { o
159159
if (scalaVersion().startsWith("2.13")) {
160160
finalOptions ++
161161
Seq(
162+
// collection types
163+
// value ArrayStack in package mutable is deprecated (since 2.13.0): Use Stack instead of ArrayStack; it now uses an array-based implementation
164+
"-Wconf:cat=deprecation&msg=value ArrayStack in package mutable is deprecated:s",
162165
// generic to immutable default
163166
// collection methods
164167
// method copyArrayToImmutableIndexedSeq in class LowPriorityImplicits2 is deprecated (since 2.13.0): implicit conversions from Array to immutable.IndexedSeq are implemented by copying; use `toIndexedSeq` explicitly if you want to copy, or use the more efficient non-copying ArraySeq.unsafeWrapArray
@@ -328,6 +331,7 @@ trait RootHailModule extends CrossScalaModule with HailModule { outer =>
328331
Rule.Relocate("org.elasticsearch.**", "is.hail.relocated.@0"),
329332
Rule.Relocate("org.json4s.**", "is.hail.relocated.@0"),
330333
Rule.Relocate("org.objectweb.**", "is.hail.relocated.@0"),
334+
Rule.Relocate("scala.collection.compat.**", "is.hail.relocated.@0"),
331335
)
332336

333337
override def scalacPluginMvnDeps: T[Seq[Dep]] = Seq(

hail/hail/src-2.12/is/hail/utils/compat/immutable/ArraySeq.scala

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package is.hail.utils.compat.immutable
22

3+
import scala.collection.compat.IterableOnce
34
import scala.collection.generic.CanBuildFrom
45
import scala.collection.mutable
56
import scala.reflect.ClassTag
@@ -13,9 +14,12 @@ object ArraySeq {
1314
def newBuilder[T: ClassTag]: mutable.Builder[T, ArraySeq[T]] =
1415
Array.newBuilder[T].mapResult(unsafeWrapArray)
1516

16-
def apply[T: ClassTag](elems: T*): ArraySeq[T] = elems match {
17+
def apply[T: ClassTag](elems: T*): ArraySeq[T] = from(elems)
18+
19+
def from[T: ClassTag](it: IterableOnce[T]): ArraySeq[T] = it match {
1720
case a: mutable.WrappedArray[T] => unsafeWrapArray(a.array)
18-
case _ => unsafeWrapArray(elems.toArray)
21+
case a: ArraySeq[T] => a
22+
case _ => unsafeWrapArray(it.toArray)
1923
}
2024

2125
def unapplySeq[T](seq: ArraySeq[T]): Some[ArraySeq[T]] = Some(seq)
@@ -24,4 +28,10 @@ object ArraySeq {
2428

2529
implicit def canBuildFrom[T: ClassTag]: CanBuildFrom[ArraySeq[_], T, ArraySeq[T]] =
2630
A.canBuildFrom
31+
32+
def fill[T: ClassTag](n: Int)(elem: => T): ArraySeq[T] =
33+
unsafeWrapArray(Array.fill(n)(elem))
34+
35+
def tabulate[T: ClassTag](n: Int)(f: Int => T): ArraySeq[T] =
36+
unsafeWrapArray(Array.tabulate(n)(f))
2737
}

hail/hail/src-2.12/is/hail/utils/compat/mutable/GrowableCompat.scala

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package is.hail.utils.compat.mutable
22

3-
import scala.collection.generic
4-
5-
trait Growable[-A] extends generic.Growable[A] {
3+
trait GrowableCompat[-A] extends Growable[A] {
64
override def +=(elem: A): this.type = addOne(elem)
75
protected def addOne(elem: A): this.type
86
def knownSize: Int = -1
Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package is.hail.utils.compat.mutable
22

3-
import scala.collection.generic
4-
5-
trait Shrinkable[-A] extends generic.Shrinkable[A] {
3+
trait ShrinkableCompat[-A] extends Shrinkable[A] {
64
override def -=(elem: A): this.type = subtractOne(elem)
75
protected def subtractOne(elem: A): this.type
86
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package is.hail.utils.compat
2+
3+
package object mutable {
4+
type Growable[-A] = scala.collection.generic.Growable[A]
5+
type Shrinkable[-A] = scala.collection.generic.Shrinkable[A]
6+
}

hail/hail/src-2.12/is/hail/utils/compat/package.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,25 @@ package is.hail.utils
33
import is.hail.utils.compat.immutable.ArraySeq
44

55
import scala.collection.compat.Factory
6+
import scala.collection.mutable.WrappedArray
67
import scala.reflect.ClassTag
78

9+
package compat {
10+
class ArrayOps[A: ClassTag](val a: WrappedArray[A]) {
11+
def sortInPlace[B >: A]()(implicit ct: ClassTag[B], ord: Ordering[B])
12+
: WrappedArray[A] = {
13+
scala.util.Sorting.stableSort(a.array.asInstanceOf[Array[B]])
14+
a
15+
}
16+
17+
def sortInPlaceBy[B](f: A => B)(implicit ord: Ordering[B]): WrappedArray[A] =
18+
sortInPlace()(implicitly, ord = ord on f)
19+
}
20+
}
21+
822
package object compat {
923
implicit def arraySeqbf[A: ClassTag](ob: ArraySeq.type): Factory[A, ArraySeq[A]] =
1024
ob.canBuildFrom[A]
25+
26+
implicit def toArrayOps[A: ClassTag](a: Array[A]): ArrayOps[A] = new ArrayOps(a)
1127
}

hail/hail/src-2.13/is/hail/utils/compat/mutable/package.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,7 @@ package is.hail.utils.compat
22

33
package object mutable {
44
type Growable[-A] = scala.collection.mutable.Growable[A]
5+
type GrowableCompat[-A] = Growable[A]
56
type Shrinkable[-A] = scala.collection.mutable.Shrinkable[A]
7+
type ShrinkableCompat[-A] = Shrinkable[A]
68
}

hail/hail/src/is/hail/annotations/OrderedRVIterator.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ object OrderedRVIterator {
1212
def multiZipJoin(
1313
sm: HailStateManager,
1414
its: IndexedSeq[OrderedRVIterator],
15-
): Iterator[BoxedArrayBuilder[(RegionValue, Int)]] = {
16-
require(its.length > 0)
15+
): Iterator[collection.IndexedSeq[(RegionValue, Int)]] = {
16+
require(its.nonEmpty)
1717
val first = its(0)
1818
val flipbooks = its.map(_.iterator.toFlipbookIterator)
19-
FlipbookIterator.multiZipJoin(
19+
FlipbookIterator.multiZipJoin[RegionValue](
2020
flipbooks.toArray,
2121
first.t.joinComp(sm, first.t).compare,
2222
)

hail/hail/src/is/hail/annotations/RegionPool.scala

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package is.hail.annotations
33
import is.hail.expr.ir.LongArrayBuilder
44
import is.hail.utils._
55

6+
import scala.collection.mutable
7+
68
object RegionPool {
79

810
def apply(strictMemoryCheck: Boolean = false): RegionPool = {
@@ -28,8 +30,8 @@ final class RegionPool private (strictMemoryCheck: Boolean, threadName: String,
2830
protected[annotations] val freeBlocks: Array[LongArrayBuilder] =
2931
Array.fill[LongArrayBuilder](4)(new LongArrayBuilder(8))
3032

31-
protected[annotations] val regions = new BoxedArrayBuilder[RegionMemory]()
32-
private[this] val freeRegions = new BoxedArrayBuilder[RegionMemory]()
33+
protected[annotations] val regions = mutable.ArrayBuffer.empty[RegionMemory]
34+
private[this] val freeRegions = mutable.ArrayStack.empty[RegionMemory]
3335
private[this] val blocks: Array[Long] = Array(0L, 0L, 0L, 0L)
3436
private[this] var totalAllocatedBytes: Long = 0L
3537
private[this] var allocationEchoThreshold: Long = 256 * 1024
@@ -103,7 +105,7 @@ final class RegionPool private (strictMemoryCheck: Boolean, threadName: String,
103105
chunkCache.freeChunkToCache(chunkPointer)
104106

105107
protected[annotations] def getMemory(size: Int): RegionMemory = {
106-
if (freeRegions.size > 0) {
108+
if (freeRegions.nonEmpty) {
107109
val rm = freeRegions.pop()
108110
rm.initialize(size)
109111
rm

hail/hail/src/is/hail/annotations/WritableRegionValue.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package is.hail.annotations
33
import is.hail.backend.HailStateManager
44
import is.hail.rvd.RVDContext
55
import is.hail.types.physical.{PStruct, PType}
6-
import is.hail.utils.compat.mutable.Growable
6+
import is.hail.utils.compat.mutable.GrowableCompat
77

88
import scala.collection.mutable.{ArrayBuffer, PriorityQueue}
99

@@ -129,7 +129,7 @@ class RegionValuePriorityQueue(
129129
}
130130

131131
class RegionValueArrayBuffer(val t: PType, region: Region, sm: HailStateManager)
132-
extends Iterable[RegionValue] with Growable[RegionValue] {
132+
extends Iterable[RegionValue] with GrowableCompat[RegionValue] {
133133

134134
val value = RegionValue(region, 0)
135135

0 commit comments

Comments
 (0)