fdietze / vectory

A 2D geometry library in Scala.
12 stars 4 forks source link

Make the used numeric type generic utilizing @specialized and spire Ring #32

Open busti opened 4 years ago

busti commented 4 years ago

Both Float and Double could be supported without any performance impact using the @specialized annotation and by depending on spire.
Spire provides the Ring typeclass, which provides basic numeric operations for generic number types.

Classes like Vec2 would roughly change to the following structure:

import scala.{specialized => sp}
import spire.algebra._

@inline final case class Vec2[@sp(Float, Double) A](x: Double, y: Double) {
  @inline def +(that: Vec2[A])(implicit s: CRing[A]) = Vec2(this.x + that.x, this.y + that.y)
}

For convenience purposes there could be type aliases to make the usage less verbose:

type Vec2f = Vec2[Float]
type Vec2d = Vec2[Double]

The generated binary sized should not be impacted, due to compiler optimization and tree shaking.

In general I just want to know if this is on the table for this library. If so I would go ahead and make the necessary changes.

fdietze commented 4 years ago

Interesting. As long as it doesn't impact performance or binary size, I'd welcome such an improvement.

Did you look at the generated JS code yet?

busti commented 4 years ago

So... I did some Work on this over the night...

So far I refactored Vec2. I also made it so that everything else compiles, using Double only.
I have excluded the structural-type constructors for now since they do not mix well with specialization and I need to investigate it further to make it work, but it is really late.

Generally it is all smooth sailing, but it does make the code really verbose.
If you want to take a look, I have pushed everything to the spire-refactor branch in my fork.
https://github.com/busti/vectory/blob/spire-refactor/vectory/src/main/scala/Vec2.scala

All tests pass, but I could not benchmark anything yet, or compare the artifact sizes (it really is late ;) )
That's all :)

fdietze commented 4 years ago

Impressive! The method signatures got more verbose, but it's still one-liners, so it's fine for me.

I'm wondering if the compiler really manages to inline all this stuff, especially for chains of vector-operations. So the generated js code would be interesting to see.

busti commented 4 years ago

The test-fullopt files have gotten larger.
880KB vs 944KB

Here is the generated code for Vec2.length for comparasion:
Without Spire

  var pos$49 = new $c_Lorg_scalactic_source_Position().init___T__T__I("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 102);
  new $c_Lorg_scalatest_freespec_AnyFreeSpecLike$FreeSpecStringWrapper().init___Lorg_scalatest_freespec_AnyFreeSpecLike__T__Lorg_scalactic_source_Position(this, "length", pos$49).$in__F0__V(new $c_sjsr_AnonFunction0().init___sjs_js_Function0((function(this$17$1) {
    return (function() {
      var pos$50 = new $c_Lorg_scalactic_source_Position().init___T__T__I("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 103);
      var prettifier$33 = $m_Lorg_scalactic_Prettifier$().default$1;
      new $c_Lorg_scalatest_matchers_must_Matchers$AnyMustWrapper().init___Lorg_scalatest_matchers_must_Matchers__O__Lorg_scalactic_source_Position__Lorg_scalactic_Prettifier(this$17$1, 25.0, pos$50, prettifier$33).mustEqual__O__Lorg_scalactic_Equality__Lorg_scalatest_compatible_Assertion(25, new $c_Lorg_scalactic_DefaultEquality().init___());
      var o$1 = $uD($g.Math.sqrt(25.0));
      var pos$51 = new $c_Lorg_scalactic_source_Position().init___T__T__I("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 104);
      var prettifier$34 = $m_Lorg_scalactic_Prettifier$().default$1;
      return new $c_Lorg_scalatest_matchers_must_Matchers$AnyMustWrapper().init___Lorg_scalatest_matchers_must_Matchers__O__Lorg_scalactic_source_Position__Lorg_scalactic_Prettifier(this$17$1, o$1, pos$51, prettifier$34).mustEqual__O__Lorg_scalactic_Equality__Lorg_scalatest_compatible_Assertion(5, new $c_Lorg_scalactic_DefaultEquality().init___())
    })
  })(this)));

With spire

  var pos$43 = new $c_Lorg_scalactic_source_Position().init___T__T__I("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 105);
  new $c_Lorg_scalatest_freespec_AnyFreeSpecLike$FreeSpecStringWrapper().init___Lorg_scalatest_freespec_AnyFreeSpecLike__T__Lorg_scalactic_source_Position(this, "length", pos$43).$in__F0__V(new $c_sjsr_AnonFunction0().init___sjs_js_Function0((function(this$15$1) {
    return (function() {
      var r$8 = $m_Lspire_implicits$().DoubleAlgebra$1;
      var a = $f_Lspire_std_DoubleIsField__times$mcD$sp__D__D__D(r$8, 3.0, 3.0);
      var b = $f_Lspire_std_DoubleIsField__times$mcD$sp__D__D__D(r$8, 4.0, 4.0);
      var o$3 = $f_Lspire_std_DoubleIsField__plus$mcD$sp__D__D__D(r$8, a, b);
      var pos$44 = new $c_Lorg_scalactic_source_Position().init___T__T__I("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 106);
      var prettifier$29 = $m_Lorg_scalactic_Prettifier$().default$1;
      new $c_Lorg_scalatest_matchers_must_Matchers$AnyMustWrapper().init___Lorg_scalatest_matchers_must_Matchers__O__Lorg_scalactic_source_Position__Lorg_scalactic_Prettifier(this$15$1, o$3, pos$44, prettifier$29).mustEqual__O__Lorg_scalactic_Equality__Lorg_scalatest_compatible_Assertion(25, new $c_Lorg_scalactic_DefaultEquality().init___());
      var r$9 = $m_Lspire_implicits$().DoubleAlgebra$1;
      $m_Lspire_implicits$();
      var a$1 = $f_Lspire_std_DoubleIsField__times$mcD$sp__D__D__D(r$9, 3.0, 3.0);
      var b$1 = $f_Lspire_std_DoubleIsField__times$mcD$sp__D__D__D(r$9, 4.0, 4.0);
      var a$2 = $f_Lspire_std_DoubleIsField__plus$mcD$sp__D__D__D(r$9, a$1, b$1);
      var o$4 = $uD($g.Math.sqrt(a$2));
      var pos$45 = new $c_Lorg_scalactic_source_Position().init___T__T__I("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 107);
      var prettifier$30 = $m_Lorg_scalactic_Prettifier$().default$1;
      return new $c_Lorg_scalatest_matchers_must_Matchers$AnyMustWrapper().init___Lorg_scalatest_matchers_must_Matchers__O__Lorg_scalactic_source_Position__Lorg_scalactic_Prettifier(this$15$1, o$4, pos$45, prettifier$30).mustEqual__O__Lorg_scalactic_Equality__Lorg_scalatest_compatible_Assertion(5, new $c_Lorg_scalactic_DefaultEquality().init___())
    })
  })(this)));

Unfortunately, inlining does not take place.
It looks a little cleaner after the closure optimization pass:
Without spire

a = (new P).f("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 102);
yn(mn(this, "length", a), G(function (b) {
    return function () {
        var c = (new P).f("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 103),
            d = D().n;
        T(R(b, 25, c, d), 25);
        c = +ba.Math.sqrt(25);
        d = (new P).f("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.",
            104);
        var e = D().n;
        return T(R(b, c, d, e), 5)
    }
}(this)));

With spire

a = (new S).g("Vec2Spec.scala",
    "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 105);
Mo(Ao(this, "length", a), K(function (b) {
    return function () {
        W();
        var c = n(3, 3), d = n(4, 4);
        c = c + d | 0;
        d = (new S).g("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 106);
        var e = J().o;
        V(T(b, c, d, e), 25);
        W();
        W();
        c = n(3, 3);
        d = n(4, 4);
        a:{
            c = c + d | 0;
            var g = 32768;
            d = 0;
            for (; ;) {
                e = d | g;
                var h = +ea.Math.pow(e, 2);
                if (h === c || 0 === g) {
                    c = e;
                    break a
                }
                0 >=
                h || h > c ? g >>= 1 : (g >>= 1, d = e)
            }
        }
        d = (new S).g("Vec2Spec.scala", "Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.", 107);
        e = J().o;
        return V(T(b, c, d, e), 5)
    }
}(this)));

So I guess that concludes that there is still a lot left to be optimized. I wonder why it computes the square root manually, maybe spire also lacks bindings to the javascript stdlib. I will see if I can make it work and ask around some more on the spire channels.
If nothing comes up, it was at least worth a try :)