Use Array[Int] instead of Array[Array[Boolean]] for [class,trait]_has_trait
#3279
Conversation
Using a Array[Long] as bitset instead of Array[Array[Boolean]]
| @@ -504,7 +592,7 @@ object Generate { | |||
| } | |||
| } | |||
|
|
|||
| private object Impl { | |||
| object Impl { | |||
There was a problem hiding this comment.
I need to move the constats that I need in to use in Lower outside of the Impl object and make them public.
| @@ -0,0 +1,26 @@ | |||
| package scala.scalanative | |||
|
|
|||
| private[scalanative] class BitMatrix private ( | |||
There was a problem hiding this comment.
This is a minimal version of our java BitSet implementation with fixed size and no bound checks.
It represents a matrix so it calculates the array index storing the matrix by rows.
| // def __get_[class,trait]_has_trait(firstid: Int, secondid: Int): Boolean = { | ||
| // val columns = meta.traits.length | ||
| // val row = firstid * columns | ||
| // val bitIndex = row + secondid | ||
| // val arrayPos = bitIndex >> AddressBitsPerWord | ||
| // val long = bits(arrayPos) | ||
| // val toShift = bitIndex & RightBits | ||
| // val toShiftLong = toShift.toLong | ||
| // val mask = 1L << toShiftLong | ||
| // val and = long & mask | ||
| // val result = and != 0L | ||
| // | ||
| // result | ||
| // } |
There was a problem hiding this comment.
I'm not sure if we need this comment. I can remove it if you think it's redundant.
|
Even though in typical usage the new implementation might not introduce any regression in performance when we isolate the benchmark to mostly type checks, then the difference can be actually observed: Anyway, I think this change should be included into the main. Probably in the future we might provide users with option to use the old implementation while acknowledging the higher binary size. In fact we might need to do something similar for virtual methods dispatch in which we use a large array to ensure constant lookup time, which however blocks capabilities of incremental compilation. BenchmarkTested with releaseFast + LTO thin trait T1
trait T2 extends T1
trait T3 extends T2
trait T4 extends T3
class C1
class C2 extends C1
class C3 extends C2
class C4 extends C3
object Test {
def main(args: Array[String]): Unit = {
var c = 0
var b = false
var i = 0
var x = 0
while (i < 1000000) {
val obj = (i % 3) match {
case 0 => new C4
case 1 => new T3 {}
case _ => new {}
}
assert(obj != null)
while (c < 1000000) {
b &&= obj.isInstanceOf[T1]
b &&= obj.isInstanceOf[T2]
b &&= obj.isInstanceOf[T3]
b &&= obj.isInstanceOf[T4]
b &&= obj.isInstanceOf[C1]
b &&= obj.isInstanceOf[C2]
b &&= obj.isInstanceOf[C3]
b &&= obj.isInstanceOf[C4]
c += 1
if (b) x += 1
}
i += 1
}
}
}
Resultsgitpod /workspace/scala-native (main) $ time ./before
real 0m0.006s
user 0m0.001s
sys 0m0.006s
gitpod /workspace/scala-native (main) $ time ./after
real 0m0.014s
user 0m0.009s
sys 0m0.005s |
|
@WojciechMazur Great that you wrote a benchmark program, so we can test the performance of a bitset with 32 bit integers and also the performance of a one-dimensional array of Booleans, which are two other options. |
That might be an interesting experiment, but I'm not sure if that would make a lot of difference. Becouse the NIR is generated after the erasure, the Scala AST might be already full of |
Btw, not a priority, but eventually I would be happy to see an option so that in release mode all |
|
Converted the matrixes to My benchmarks:
|
lolgab
changed the title
Array[Long] instead of Array[Array[Boolean]] for [class,trait]_has_traitArray[Int] instead of Array[Array[Boolean]] for [class,trait]_has_trait
lolgab
changed the title
Array[Int] instead of Array[Array[Boolean]] for [class,trait]_has_traitArray[Int] instead of Array[Array[Boolean]] for [class,trait]_has_trait
|
Thank you for introducing me to |
…]_has_trait` (#3279) * Efficient trait_has_trait using a Array[Int] as bitset instead of Array[Array[Boolean]]
…]_has_trait` (#3279) * Efficient trait_has_trait using a Array[Int] as bitset instead of Array[Array[Boolean]]
Results
Here the results with the Ember hello world.
the binary size is reduced by ~
4.9 MBand the generatedout.llfile is reduced by55 MBThe runtime perfomance seems unchanged: