- Notifications
You must be signed in to change notification settings - Fork238
Qualified methods#993
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…eDefinition/MethodDefinition may be used.
iskiselev commentedJun 9, 2016
As@kg supposed, this PR really increase memory usage. I've not supposed that memory usage increase will be so big. Will be glad for any help/suggestions what we can do with it |
| $jsilcore.CanFixUpEnumInterfaces=false; | ||
| JSIL.FixupInterfaces=function(publicInterface,typeObject){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Main changes is inside this method. It now calculates proper virtual member implementation for each base type, not only implemented interfaces.
Check is stricter now - method should be virtual and for auto-binding by name to interface class must explicitly declare that interface in it's own declaration.
iskiselev commentedJun 10, 2016
@kg, why do we cache MethodSignatures/TypeRefernce per type instead of caching them per assembly? |
kg commentedJun 10, 2016
@iskiselev Mostly because it was easier. I see no reason not to do it assembly-wide. |
iskiselev commentedJun 15, 2016
I've broke#624 with null arg to instance method with this change. I believe it's OK, as nobody should really write such code. We still may fix it later. |
iskiselev commentedJun 16, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Looks like I'm nearly done with it. The only left is moving Call method creation from MethodSignature to InterfaceMethod and small cleanup of code inside MethodSignature / InterfaceMethod.
|
iskiselev commentedJun 17, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@kg, I think we could not use MethodSignature/InterfaceMethod Call/CallNonVirtual/CallStatic at all. We will use for non-variance call: And for variance calls: I believe it should not have any performance issues, but will improve performance/memory footprint as we will not create Call method closures. What do you think? |
kg commentedJun 17, 2016
It looks gross, but it might be considerably better for performance, so maybe it's worth it. It does solve the this-reference binding problem and it eliminates most of the need for closures, which is nice. |
iskiselev commentedJun 17, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
With JavaScript you can get performance problem nearly anywhere. Chrome | Accessor: 580 ms first calls, 130 ms next calls (not always, sometimes it stays 580 forever) Firefox | Accessor: 210 ms Edge | Accessor: 800-900 ms @kg, have you any ideas what we could do with it? Also I'm interesting how arguments count will affect this numbers. Oh, Edge works in real-life better than in Test. Strange.
Update: test result updated based onchakra-core/ChakraCore#1153 (comment) |
iskiselev commentedJun 17, 2016
Looks like the only way to achieve normal performance would be calculate function overload name using some stable algorithm and hardcode them on translation. Otherwise we will be much slower comparing to .Net. |
iskiselev commentedJun 17, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Results for stable: |
iskiselev commentedJun 17, 2016
We definitely need report bug to Chackra, as both Signature and Stable takes too long, Even Prototype takes longer, then others. I just not sure how it should be described. |
kg commentedJun 17, 2016
The solution here is probably (ugh) to move to shipping some sort of bytecode in Release builds, and then translate the bytecode to JS at loadtime. That translated JS can have all the keys hardcoded as |
iskiselev commentedJun 17, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@kg, tried create small sample with run-time generated functions and added it to sample: As I can see, Chakra works bad with run-time generated functions (will create simplified sample and report, if it is not reported yet). It's interesting for me, why Firefox works 3 times slower comparing to Signatures case. But still it works fast enough to try it. |
kg commentedJun 17, 2016
My long-term strategy for bytecode would be to move things around such that the existing ASTEmitter can run in the browser, and then we ship a serialized version of the JS AST to it over the wire. For debug builds we would just run the ASTEmitter up front and save the output .js files. |
iskiselev commentedJun 17, 2016
It really sounds great, but I need to solve problem right now. Looks like hacking AST would be the fastest way to do it. |
iskiselev commentedJun 17, 2016
Safari also likes Stable/Generated approaches. They run faster than any else. |
kg commentedJun 18, 2016
If this is blocking you right now, it sounds like a short-term solution is I'm okay with a stable compile-time name generation approach as long as we On 17 June 2016 at 15:33, Igor Kiselevnotifications@github.com wrote:
|
iskiselev commentedJun 18, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Right now I'm planning use "Generated" approach. So, each function will be written as sting with includes of run-time calculated signatures. Something like: I know, it really looks ugly, but I believe that it could be done in just few days and may give big performance improvement. |
kg commentedJun 18, 2016
Yikes. Well, as long as it's an option... I'd like to find a better On 17 June 2016 at 18:17, Igor Kiselevnotifications@github.com wrote:
|
iskiselev commentedJun 18, 2016
Really I'll use it only for release build, but if it gives me 2-5 times performance improvement I'll accept it. If performance improvement will be less than 2 times, I'll not use it. |
kg commentedJun 18, 2016
Yeah if it's that much faster it's worth it. Ideally we can just improve As-is all that concatenation means you can't effectively ctrl-f in the On 17 June 2016 at 18:21, Igor Kiselevnotifications@github.com wrote:
|
iskiselev commentedJun 18, 2016
Templates would not help us with ctrl-f, as we plan to modify string. |
iskiselev commentedJun 18, 2016
If we will talk about "Stable" names approach, the main problem now is that I can't found way to encode function for interfaces. If we'll find such way, it would be also pretty good option. |
This PR is not yet finished, but I'd like to start discussions for it. Main goal here is implement#991 andfix#185,#623.
Instead of MethodSignature for method calls used InterfaceMethod (that is just signature+name+source type).
Also fixed huge number of small bugs that was revealed during implementation (mostly with very edge cases, so they was not noticed before).