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

Native indexOf() for js.Array #1447

Closed
tindzk opened this issue Jan 12, 2015 · 4 comments
Closed

Native indexOf() for js.Array #1447

tindzk opened this issue Jan 12, 2015 · 4 comments
Labels
as-designed The observed behavior is as-designed, it need not be fixed.

Comments

@tindzk
Copy link

tindzk commented Jan 12, 2015

indexOf() is implemented in js.Array using an implicit although JavaScript already provides an implementation out of the box. If I understand correctly, implicits incur a run-time overhead when used on native JavaScript collection types.

@sjrd
Copy link
Member

sjrd commented Jan 12, 2015

The implicits are inlined away by the Scala.js optimizer. So no, there is no runtime overhead.

@sjrd sjrd closed this as completed Jan 12, 2015
@tindzk
Copy link
Author

tindzk commented Jan 12, 2015

Yes, implicits are inlined, but it still rolls out its own version of indexOf(). I just verified with the following code:

  def testA(): Int = {
    val arr = js.Array(1)
    arr.indexOf(1)
  }

  def testB(): Int = {
    val arr = js.Array(1)
    arr
      .asInstanceOf[js.Dynamic]
      .indexOf(1.asInstanceOf[js.Dynamic])
      .asInstanceOf[Int]
  }

This results in:

ScalaJS.c.Lorg_widok_todomvc_Main$.prototype.testA__I = (function() {
  var arr = [1];
  var len = ScalaJS.uI(arr["length"]);
  var i = 0;
  while (true) {
    if ((i < len)) {
      var index = i;
      var arg1 = arr[index];
      var jsx$1 = (!ScalaJS.m.sr_BoxesRunTime$().equals__O__O__Z(1, arg1))
    } else {
      var jsx$1 = false
    };
    if (jsx$1) {
      i = ((1 + i) | 0)
    } else {
      break
    }
  };
  var n = i;
  return ((n >= ScalaJS.uI(arr["length"])) ? (-1) : n)
});

ScalaJS.c.Lorg_widok_todomvc_Main$.prototype.testB__I = (function() {
  var arr = [1];
  return ScalaJS.uI(arr["indexOf"](1))
});

Ideally, it should use the native indexOf() for testA, too.

@sjrd
Copy link
Member

sjrd commented Jan 12, 2015

Ideally, it should use the native indexOf() for testA, too.

No, ideally it would do exactly what it does. Its own rolled out version of indexOf is faster than the "native" one. And it follows the Scala semantics of ==, which the "native" version doesn't do.

@tindzk
Copy link
Author

tindzk commented Jan 12, 2015

Thanks for pointing this out. Intuitively, I would have assumed the native version to be faster though, but apparently indexOf() also does a has-property check which makes it slower (see also http://jsperf.com/js-for-loop-vs-array-indexof).

tindzk added a commit to widok/widok that referenced this issue Jan 12, 2015
@sjrd sjrd added the as-designed The observed behavior is as-designed, it need not be fixed. label Jul 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as-designed The observed behavior is as-designed, it need not be fixed.
Projects
None yet
Development

No branches or pull requests

2 participants