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

Function invocation slow when stored by run-time calculated key #1153

Closed
iskiselev opened this issue Jun 17, 2016 · 10 comments
Closed

Function invocation slow when stored by run-time calculated key #1153

iskiselev opened this issue Jun 17, 2016 · 10 comments

Comments

@iskiselev
Copy link

I'm working on sq/JSIL#993 and try to find a way for fast invocation of method stored in object by run-time calculated key. I've write some tests to found a way for finding proper JS form, and looks like Edge works much slower than other browsers. Please see my results in table sq/JSIL#993 (comment).
What is really bad, it works very slow (30-50 times) for last too options (Stable/Generated), which are the best for other browsers, and really only way for implementing dynamic method invocation.
Here are my test cases.

@ianwjhalliday
Copy link
Collaborator

@rajatd @LouisLaf

@dilijev dilijev added the Bug label Jun 22, 2016
@iskiselev
Copy link
Author

Could anybody suggest me any any workarounds that may improve speed of sample code?

@rajatd
Copy link
Contributor

rajatd commented Jun 23, 2016

Hi @iskiselev ,

Thanks for reporting this issue. I looked at stable.html and observed that it was calling a method invoked via [] operator and the method name starts with 0. We try to convert constructs of the form obj[foo]() to obj.foo() as we generate better code for the latter. But in this case we don't do the conversion since the property name starts with a number and it is a syntax error to have the right hand side of a dot operator to be starting with a number.

A workaround for this would be to set your property as "$BaseMethod$void=void" + key instead of key + "$BaseMethod$void=void". Would that still meet your requirements?

Looking at generated.html now.

Btw, I tried the other variants too, and Edge seems to be on par with Chrome in prototype.html.
Edge: 14
Chrome: 14

@iskiselev
Copy link
Author

iskiselev commented Jun 23, 2016

@rajatd, thank you for your advice. I've updated my test case with moving key to the end of propertyName. It's much better now.
Unfortunately, using of prototype will not be possible for virtual function invocation, so my real options are Signatures and Generate.
It's very strange for me, that Signatures version works 8 time faster than Generated version - as both of them are using runtime function creation (and Generated works fastest way in all other script engines).
Update: Really, I hoped that Generated will work with nearly same speed as Stable, same as all other engines.

@rajatd
Copy link
Contributor

rajatd commented Jun 24, 2016

@iskiselev, I was looking into Generated case with @LouisLaf. Like stable, it too invokes a method via [] operator but this time, it has a concat inside [ ]. Unfortunately, we don't generate optimal code for looking up property names that come from string operations (such as concat in this case). We discussed what would be required to optimize such cases in the JIT backend. Basically, the JIT will have to detect string literals and generate code assuming that the [ ] operator was actually a dot operator.

Also, could you provide an overview of what is it that you're trying to achieve through function invocation using runtime calculated key? It'll be helpful for us to better understand the issue and maybe we could come up with a workaround.

@iskiselev
Copy link
Author

@rajatd, really Generated case tried to avoid using of concat inside [], as I already found that it is slow.
There I generate function, that looks like (from developer tools):

function anonymous() {
"use strict"
var key = "$BaseMethod$void=void1466731069636"; this[key]();Program.I += 2;
}

I do concatenation of string to form key in string before invoking function constructor.

When we translate .Net sources, we may have virtual functions, besides function with same name may have several overload. In run-time we assign id for each .Net type representation, and form real name for methods.
Let's say we have:

class Base
{
  virtual void Method(string a) {}
  void Method(int a) {}
}
class Derived : Base
{
  override void Method(string a) {}
}

Suppose in run-time we assigned next ID:
string - 1
int - 2
Base - 3
Derived - 4

Then, we will define on Base prototype next methods:

$I3$Method$1=void - for virtual call Base.Method(string a)
$i$I3$Method$1=void - for non-virtual call Base.Method(string a)
$I3$Method$2=void - for virtual call Base.Method(int a)
$i$I3$Method$2=void - for non-virtual call Base.Method(int a)

And on Derived prototype:

$I4$Method$1=void - for virtual call Derived.Method(string a)
$i$I4$Method$1=void - for non-virtual call Derived.Method(string a)

For invocation, we encode proper method signature into special object - something like:

var method = $asm01.Base.Of(JSIL.MethodSignature($jsilcore.System.Void, [$jsilcore.System.String]));

There, $asm01 and $jsilcore - are objects that contains our .Net types from corresponding assembly.
Later we may either use: method.Call(baseObj, "someString"), but it will add additional middle - layer for Call method, or use: baseObj[method.key]("someString")
Now, I'm trying to find fastest way for such invocation (we already cache signatures object).

@iskiselev
Copy link
Author

iskiselev commented Jun 27, 2016

When I changed generated function, to call function through dot-expression, it start working OK in Chakra (see generateForChakra.html). So, if we are talking about my previous sample:

function anonymous() {
"use strict"
// var key = "$BaseMethod$void=void1466731069636";
// this[key]();
this.$BaseMethod$void$void1466731069636();
Program.I += 2;
}

It feels as lack in Chakra function optimization, that it was unable to found effective constant expression and make dot-optimization itself.
At least now I have some workaround to apply (I'm not fully happy with it, but at least it works).

It steal works slower than other browsers, and it steal slower than "Stable" version.

@rajatd
Copy link
Contributor

rajatd commented Jun 30, 2016

@iskiselev the reason for remaining perf difference between Edge and other browsers on generatedForChakra is most likely that we don't inline calls in functions created via Function("..."). For the string concat case, we have ideas of how to make it more performant but our resources are invested in higher priority perf work items at this point.
I'm going to keep the issue open for tracking that work.

@EdMaurer EdMaurer removed the Bug label Nov 2, 2016
@EdMaurer
Copy link
Contributor

EdMaurer commented Nov 2, 2016

Not a bug as semantics are correct, but performance could be improved.

@EdMaurer EdMaurer closed this as completed Nov 2, 2016
@dilijev
Copy link
Contributor

dilijev commented Nov 2, 2016

If it's still an actionable issue, do we want to close this? We could move it to the Backlog milestone instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants