Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reimplementing longs based on Scala.js ones #7

Merged
merged 22 commits into from Mar 23, 2018

Conversation

Projects
None yet
2 participants
@jtulach
Copy link
Owner

commented Mar 18, 2018

Finally I found some time and after watching Sébastien's talk at VMM 2017 Prague I managed to use Scala.js longs in Bck2Brwsr VM. I've started with a small Scala program:

import scala.scalajs.js
import scala.scalajs.js.annotation._
import scala.scalajs.runtime.RuntimeLong

@JSExportAll
@JSExportTopLevel( "Module" )
object Module {
  def to64(lo: Int, hi: Int): Any = new RuntimeLong(lo, hi)
  def high32(a: RuntimeLong): Int = a.hi;
  def low32(a: RuntimeLong): Int = a.lo;
  def fromDouble(a: Double): Long = a.toLong;
  def toDouble(a: Long): Double = a;
  def add64(a: Long, b: Long): Long = a + b;
  def sub64(a: Long, b: Long): Long = a - b;
  def mul64(a: Long, b: Long): Long = a * b;
  def div64(a: Long, b: Long): Long = a / b;
  def mod64(a: Long, b: Long): Long = a % b;
  def and64(a: Long, b: Long): Long = a & b;
  def or64(a: Long, b: Long): Long = a | b;
  def xor64(a: Long, b: Long): Long = a ^ b;
  def neg64(a: Long): Long = -a;
  def shl64(a: Long, n : Int): Long = a << n;
  def shr64(a: Long, n : Int): Long = a >> n;
  def ushr64(a: Long, n : Int): Long = a >>> n;
  def compare64(a: Long, b: Long): Int = {
    if (a == b) return 0;
    if (a < b) return -1;
    return 1;
  }
}

I compiled it to JavaScript and then I included the generated code in Bck2Brwsr's java_lang_Number emulation. I can confirm that the Scala.js long emulation speeds my benchmarks in LongSieveTest few times.

@sjrd, are you OK, if I include the scala.js code this way? Is the attribution enough? Anything else I could improve?

Thanks in advance for your review.

@sjrd

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2018

You're very welcome to use the code as is.

That said, it's clearly not the most efficient way to do it. You should consider taking the generated code of $c_sjsr_RuntimeLong and its companion $c_sjsr_RuntimeLong$ and directly use those as your class of Longs. You will avoid 3 or 4 indirections by doing so (and a lot of code size). It's definitely more work, though :(

In any case, make sure you use the following sbt setting when you generate the code:

scalaJSLinkerConfig ~= { _.withSemantics(_.optimized) }

this will ensure that the optimizer produces the best code it can. By default it generates more checks that are intended for debugging. It might not make a huge difference for this particular use case, though, but it's good practice anyway.

@sjrd

This comment has been minimized.

Copy link
Contributor

commented Mar 18, 2018

In your code you have

  def compare64(a: Long, b: Long): Int = {
    if (a == b) return 0;
    if (a < b) return -1;
    return 1;
  }

You should use java.lang.Long.compare instead. It is intrisified by the optimizer:

def compare64(a: Long, b: Long): Int = java.lang.Long.compare(a, b)

Similarly, j.l.Long.divideUnsigned and j.l.Long.remainderUnsigned receive some special treatment.

@jtulach

This comment has been minimized.

Copy link
Owner Author

commented Mar 21, 2018

Using optimized semantics without closure compiler from scalor may not be completely easy. Probably better if I switch to sbt - at least I will be able to follow the tutorials. I've managed to generate about two thousands of code via sbt run in a97991a

@jtulach jtulach force-pushed the ScalaLongs branch from 6def7bc to a97991a Mar 21, 2018

@sjrd

This comment has been minimized.

Copy link
Contributor

commented on rt/vm/longs/project/plugins.sbt in a97991a Mar 21, 2018

You'll get more performance still by using the latest 1.x milestone, currently 1.0.0-M3.

There have been significant performance improvements in 1.x, including ones that affect RuntimeLong

@sjrd

This comment has been minimized.

Copy link
Contributor

commented on .gitignore in a97991a Mar 21, 2018

project/project/ could contain sources. What you want to ignore is project/project/target/.

Note that you can ignore target/ in any subdirectory with just

target/

without leading /. Looks like you could use that.

@jtulach
Copy link
Owner Author

left a comment

I am satisfied. Almost thousand lines of manually written code has been replaced by two thousands of sparse generated code based on forty lines of Scala. Nice.

Bck2Brwsr got faster too. I see about 50% speed up in the (heavily long computation oriented) JBox2D example.

Thank you Sébastien!

function $h_s_util_hashing_MurmurHash3$() {
/*<skip>*/
}
$h_s_util_hashing_MurmurHash3$.prototype = $c_s_util_hashing_MurmurHash3$.pro

This comment has been minimized.

Copy link
@jtulach

jtulach Mar 22, 2018

Author Owner

@sjrd, if there is an easy way to avoid these hashing functions to be generated, I happily use that.

This comment has been minimized.

Copy link
@sjrd

sjrd Mar 22, 2018

Contributor

I'm afraid there isn't. They are transitively needed by the hashCode function of JavaScriptException, which is used in Throwable.fillInStackTrace. It's a bunch of internal stuff on which you don't have control, as a user of Scala.js.

The thing is that Scala.js has never been optimized for "tiny" programs like yours, so the constant cost of an almost empty program is not something we've tried to reduce. It could happen one day.

There is a hard way, though, which would be to fork Scala.js for your specific use case, and define a trivial

override def hashCode(): Int = exception.hashCode()

in JavaScriptException.

In fact, maybe we should do that anyway in the core ...


// Instance tests for array of primitives

var $isArrayOf_Z = $makeIsArrayOfPrimitive($d_Z);

This comment has been minimized.

Copy link
@jtulach

jtulach Mar 22, 2018

Author Owner

All $isArrayXYZ functions are just generated, but never used (except in the RTTI part).

This comment has been minimized.

Copy link
@sjrd

sjrd Mar 22, 2018

Contributor

Yes, those are part of the hand-written JS code which is not analyzed for actual usage. Normally we rely on Closure in fullOptJS to get rid of those little details. It is good at dealing with top-level functions (less so at dealing with methods in hierarchies, which is why we do that ourselves).


@JSExportAll
@JSExportTopLevel( "Longs" )
object Module {

This comment has been minimized.

Copy link
@jtulach

jtulach Mar 22, 2018

Author Owner

This file is certainly easier to manage than the original java_lang_Number.js.

this.stackTraceStateInternal$1 = null;
this.stackTrace$1 = null
}
$c_jl_Exception.protot

This comment has been minimized.

Copy link
@jtulach

jtulach Mar 22, 2018

Author Owner

If there is an easy way to get rid of the MurmurHash code, I'd be thankful for using it as well. But yes, I know JVM is hash oriented system.

this.stackTraceStateInternal$1 = null;
this.stackTrace$1 = null
}
$c_jl_Exception.protot

This comment has been minimized.

Copy link
@sjrd

sjrd Mar 22, 2018

Contributor

You can significantly speed up that test by turning it around:

if (typeof value !== 'number') {

You know that at this point value is either a number or a RuntimeLong. Testing whether it's a number is much faster than testing whether it is a RuntimeLong.

Btw, shouldn't your compiler statically know beforehand which one it is, and avoid that test completely?

This comment has been minimized.

Copy link
@jtulach

jtulach Mar 23, 2018

Author Owner

It does, if the code is coming from the JVM. However the Bck2BrwsrVM <-> JS interop allowed to pass any number as long to any function accepting long. Clueless approach, I know, and this check is result of that.

object Module {
def to64(lo: Int, hi: Int): Any = new RuntimeLong(lo, hi)
def high32(a: RuntimeLong): Int = a.hi;
def low32(a: RuntimeLong): Int = a.lo;

This comment has been minimized.

Copy link
@sjrd

sjrd Mar 22, 2018

Contributor

In order not to rely on the API of RuntimeLong at the Scala.js level (which is not really a public API), you can write those three methods as:

def to64(lo: Int, hi: Int): Long = (lo.toLong & 0xffffffffL) | (hi.toLong << 32)
def high32(a: Long): Int = (a >>> 32).toInt
def low32(a: Long): Int = a.toInt

The Scala.js optimizer is good enough to optimize away all of that encoding, so those are compiled down to:

$c_Lhelloworld_HelloWorld$.prototype.to64__I__I__J = (function(lo, hi) {
  return new $c_sjsr_RuntimeLong(lo, hi)
});
$c_Lhelloworld_HelloWorld$.prototype.high32__J__I = (function(a) {
  var lo = a.hi$2;
  return lo
});
$c_Lhelloworld_HelloWorld$.prototype.$$js$exported$meth$low32__J__O = (function(a) {
  return a.lo$2
});

This comment has been minimized.

Copy link
@jtulach

jtulach Mar 23, 2018

Author Owner

The longs project isn't part of the build, so I guess this is not that urgent. I know that RuntimeLong is an implementation detail; should it ever disappear, I do the bit manipulation as you suggest.

@jtulach jtulach self-assigned this Mar 23, 2018

override def fillInStackTrace(): Throwable = {
this
}
override def hashCode(): Int = 1973;

This comment has been minimized.

Copy link
@jtulach

jtulach Mar 23, 2018

Author Owner

This works for me and removes the generated MurmurHash3 code, @sjrd. Scala.js could do something similar (with different constant, of course), or by returning getMessage().hashCode() for example.

@jtulach jtulach merged commit 9d9cdd3 into master Mar 23, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jtulach jtulach deleted the ScalaLongs branch Mar 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.