Use JSC C API for better invocation speed #1037

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
8 participants
@robertjpayne
Contributor

robertjpayne commented Apr 27, 2015

This converts the existing JSEvaluateScript call for require('<ModuleName>').<MethodName>.apply(null, <args>); to native JSC C API methods which shaves off about 10-15% of invocation time on average, I used pretty primitive profiling methods to track the minimum, maximum and average invocation time so would appreciate any extra eyes on the performance.

If the argument count is zero the method is invoked directly with no arguments, if the argument count is 1 it's invoked directly with just that argument. If there is more than 1 argument then apply is used and the arguments are passed as the second parameter.

Ensured all existing tests pass and instruments leaks shows nothing is leaking.

Use JSC C API for extra invocation speed
This converts the existing JSEvaluateScript call for `require('<ModuleName>').<MethodName>.apply(null, <args>);` to native JSC C API methods which shaves off about 10-15% of invocation time on average.

@robertjpayne robertjpayne changed the title from Use JSC C API for extra invocation speed to Use JSC C API for better invocation speed Apr 27, 2015

@ide

This comment has been minimized.

Show comment
Hide comment
@ide

ide Apr 27, 2015

Collaborator

Nice find...

Have you looked into the Obj-C API? I believe it would slim the code down to 1/4 its size, partly because it's more succinct and also because you could write [module invokeMethod:methodName withArguments:arguments]. (This would also fix what I think is a bug in your diff where thisObject is missing or referring to the global object instead of the module object.)

Collaborator

ide commented Apr 27, 2015

Nice find...

Have you looked into the Obj-C API? I believe it would slim the code down to 1/4 its size, partly because it's more succinct and also because you could write [module invokeMethod:methodName withArguments:arguments]. (This would also fix what I think is a bug in your diff where thisObject is missing or referring to the global object instead of the module object.)

@robertjpayne

This comment has been minimized.

Show comment
Hide comment
@robertjpayne

robertjpayne Apr 27, 2015

Contributor

@ide I had looked into it but saw a previous issue ticket that stated the Objective-C API was avoided due to corrupted memory issues.

Mind elaborating on the thisObject bug?

Contributor

robertjpayne commented Apr 27, 2015

@ide I had looked into it but saw a previous issue ticket that stated the Objective-C API was avoided due to corrupted memory issues.

Mind elaborating on the thisObject bug?

@ide

This comment has been minimized.

Show comment
Hide comment
@ide

ide Apr 27, 2015

Collaborator

Left some comments on the diff regarding the thisObject bug.

I took a look at the issue mentioning memory corruption and it sounded like the problem was with JSExport or perhaps the wrapper layer between Obj-C and JS objects. There's also some chance that the issue was due to bridged native objects getting freed at non-deterministic times if they weren't registered with the GC (just a guess). For this code at hand the Obj-C layer is straightforward without JSExport magic so I expect it not to have strange bugs.

Collaborator

ide commented Apr 27, 2015

Left some comments on the diff regarding the thisObject bug.

I took a look at the issue mentioning memory corruption and it sounded like the problem was with JSExport or perhaps the wrapper layer between Obj-C and JS objects. There's also some chance that the issue was due to bridged native objects getting freed at non-deterministic times if they weren't registered with the GC (just a guess). For this code at hand the Obj-C layer is straightforward without JSExport magic so I expect it not to have strange bugs.

@robertjpayne

This comment has been minimized.

Show comment
Hide comment
@robertjpayne

robertjpayne Apr 27, 2015

Contributor

@ide I'm looking at the thisObject stuff, because I'm pretty sure I tried what you're mentioning and found the behavior to be unexpected ( the C API seemed to be acting up ) but I'll double check.

As for the Objective-C API, it would presumably work just fine, but it'll be less performant due to how JSC internally maps objects and stuff. Since this method is executed every frame would it be best to keep it as performant as possible? I suppose the slowest bit really is the JSON conversion that will need to remain either way.

Contributor

robertjpayne commented Apr 27, 2015

@ide I'm looking at the thisObject stuff, because I'm pretty sure I tried what you're mentioning and found the behavior to be unexpected ( the C API seemed to be acting up ) but I'll double check.

As for the Objective-C API, it would presumably work just fine, but it'll be less performant due to how JSC internally maps objects and stuff. Since this method is executed every frame would it be best to keep it as performant as possible? I suppose the slowest bit really is the JSON conversion that will need to remain either way.

@robertjpayne

This comment has been minimized.

Show comment
Hide comment
@robertjpayne

robertjpayne Apr 27, 2015

Contributor

@ide tidied up the thisObject, doesn't seem to affect anything but it's at least correct now.

Contributor

robertjpayne commented Apr 27, 2015

@ide tidied up the thisObject, doesn't seem to affect anything but it's at least correct now.

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Apr 27, 2015

Contributor

This is awesome! I'm sure that there's a lot of low hanging fruits in terms of performance at the bridge level :)

You mention a 10-15% reduction, mind sharing your methodology so that we can reproduce it?

Contributor

vjeux commented Apr 27, 2015

This is awesome! I'm sure that there's a lot of low hanging fruits in terms of performance at the bridge level :)

You mention a 10-15% reduction, mind sharing your methodology so that we can reproduce it?

@robertjpayne

This comment has been minimized.

Show comment
Hide comment
@robertjpayne

robertjpayne Apr 27, 2015

Contributor

@vjeux The 10-15% was very rudimentary profiling:

I added:

static CFTimeInterval lowInterval = CGFLOAT_MAX;
static CFTimeInterval avgInterval = 0;
static CFTimeInterval highInterval = CGFLOAT_MIN;

CFTimeInterval before = CACurrentMediaTime();

.... bridge code ....

CFTimeInterval after = CACurrentMediaTime();
CFTimeInterval elapsed = after - before;

lowInterval = MIN(lowInterval, elapsed);
avgInterval = (avgInterval + elapsed) / 2.0;
highInterval = MAX(highInterval, elapsed);

printf("%f, %f, %f\n", lowInterval, avgInterval, highInterval);

Running on device I was looking mostly at the lowInterval and it's "idle" performance was 10-15% lower than it's counter part. I should probably look the interval markers only when payloads are being passed (such as scroll events).

Contributor

robertjpayne commented Apr 27, 2015

@vjeux The 10-15% was very rudimentary profiling:

I added:

static CFTimeInterval lowInterval = CGFLOAT_MAX;
static CFTimeInterval avgInterval = 0;
static CFTimeInterval highInterval = CGFLOAT_MIN;

CFTimeInterval before = CACurrentMediaTime();

.... bridge code ....

CFTimeInterval after = CACurrentMediaTime();
CFTimeInterval elapsed = after - before;

lowInterval = MIN(lowInterval, elapsed);
avgInterval = (avgInterval + elapsed) / 2.0;
highInterval = MAX(highInterval, elapsed);

printf("%f, %f, %f\n", lowInterval, avgInterval, highInterval);

Running on device I was looking mostly at the lowInterval and it's "idle" performance was 10-15% lower than it's counter part. I should probably look the interval markers only when payloads are being passed (such as scroll events).

@robertjpayne

This comment has been minimized.

Show comment
Hide comment
@robertjpayne

robertjpayne Apr 27, 2015

Contributor

It's actually pretty hard to formulate a good test plan to profile this in a "fair" way. I think mostly what I was going off was the fastest execution time marked by the low interval. Since a lot of the messages going over the bridge are nearly identical ( scroll events for instance ) it was a decent mark to play around for awhile.

I've tested it again only measuring messages that have more than 300 JSON characters in length and it's still measuring 10-15% faster on my 4S

Contributor

robertjpayne commented Apr 27, 2015

It's actually pretty hard to formulate a good test plan to profile this in a "fair" way. I think mostly what I was going off was the fastest execution time marked by the low interval. Since a lot of the messages going over the bridge are nearly identical ( scroll events for instance ) it was a decent mark to play around for awhile.

I've tested it again only measuring messages that have more than 300 JSON characters in length and it's still measuring 10-15% faster on my 4S

@robertjpayne

This comment has been minimized.

Show comment
Hide comment
@robertjpayne

robertjpayne Apr 27, 2015

Contributor

@vjeux here's the code I have in place now, trying to formulate how I could make this more predictable / comparable but the nature of the bridge is hard.

    static CFTimeInterval lowInterval = CGFLOAT_MAX;
    static CFTimeInterval totalInterval = 0;
    static NSUInteger totalIntervalCount = 0;
    static CFTimeInterval highInterval = CGFLOAT_MIN;

    CFTimeInterval before = CACurrentMediaTime();

    JSValueRef errorJSRef = NULL;
    JSValueRef resultJSRef = NULL;
    JSGlobalContextRef contextJSRef = JSContextGetGlobalContext(strongSelf->_context.ctx);
    JSObjectRef globalObjectJSRef = JSContextGetGlobalObject(strongSelf->_context.ctx);

#pragma mark - C API

//    NSError *error;
//    NSString *argsString = (arguments.count == 1) ? RCTJSONStringify(arguments[0], &error) : RCTJSONStringify(arguments, &error);
//    if (!argsString) {
//      RCTLogError(@"Cannot convert argument to string: %@", error);
//      onComplete(nil, error);
//      return;
//    }
//    
//    // get require
//    JSStringRef requireNameJSStringRef = JSStringCreateWithUTF8CString("require");
//    JSValueRef requireJSRef = JSObjectGetProperty(contextJSRef, globalObjectJSRef, requireNameJSStringRef, &errorJSRef);
//    JSStringRelease(requireNameJSStringRef);
//    
//    if (requireJSRef != NULL && errorJSRef == NULL) {
//      
//      // get module
//      JSStringRef moduleNameJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)name);
//      JSValueRef moduleNameJSRef = JSValueMakeString(contextJSRef, moduleNameJSStringRef);
//      JSValueRef moduleJSRef = JSObjectCallAsFunction(contextJSRef, (JSObjectRef)requireJSRef, NULL, 1, (const JSValueRef *)&moduleNameJSRef, &errorJSRef);
//      JSStringRelease(moduleNameJSStringRef);
//      
//      if (moduleJSRef != NULL && errorJSRef == NULL) {
//        
//        // get method
//        JSStringRef methodNameJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)method);
//        JSValueRef methodJSRef = JSObjectGetProperty(contextJSRef, (JSObjectRef)moduleJSRef, methodNameJSStringRef, &errorJSRef);
//        JSStringRelease(methodNameJSStringRef);
//        
//        if (methodJSRef != NULL && errorJSRef == NULL) {
//          
//          // direct method invoke with no arguments
//          if (arguments.count == 0) {
//            resultJSRef = JSObjectCallAsFunction(contextJSRef, (JSObjectRef)methodJSRef, (JSObjectRef)moduleJSRef, 0, NULL, &errorJSRef);
//          }
//          // direct method invoke with 1 argument
//          else if(arguments.count == 1) {
//            JSStringRef argsJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)argsString);
//            JSValueRef argsJSRef = JSValueMakeFromJSONString(contextJSRef, argsJSStringRef);
//            resultJSRef = JSObjectCallAsFunction(contextJSRef, (JSObjectRef)methodJSRef, (JSObjectRef)moduleJSRef, 1, &argsJSRef, &errorJSRef);
//            JSStringRelease(argsJSStringRef);
//          }
//          // apply invoke with array of arguments
//          else {
//            // get apply
//            JSStringRef applyNameJSStringRef = JSStringCreateWithUTF8CString("apply");
//            JSValueRef applyJSRef = JSObjectGetProperty(contextJSRef, (JSObjectRef)methodJSRef, applyNameJSStringRef, &errorJSRef);
//            JSStringRelease(applyNameJSStringRef);
//            
//            if (applyJSRef != NULL && errorJSRef == NULL) {
//              // invoke apply
//              JSStringRef argsJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)argsString);
//              JSValueRef argsJSRef = JSValueMakeFromJSONString(contextJSRef, argsJSStringRef);
//              
//              JSValueRef args[2];
//              args[0] = JSValueMakeNull(contextJSRef);
//              args[1] = argsJSRef;
//              
//              resultJSRef = JSObjectCallAsFunction(contextJSRef, (JSObjectRef)applyJSRef, (JSObjectRef)moduleJSRef, 2, args, &errorJSRef);
//              JSStringRelease(argsJSStringRef);
//            }
//          }
//        }
//      }
//      
//    }

#pragma mark - EVALUATE SCRIPT

      NSError *error;
      NSString *argsString = RCTJSONStringify(arguments, &error);
      if (!argsString) {
        RCTLogError(@"Cannot convert argument to string: %@", error);
        onComplete(nil, error);
        return;
      }

    NSString *script = [NSString stringWithFormat:@"require('%@').%@.apply(null, %@);", name, method, argsString];
    JSStringRef scriptJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)script);
    resultJSRef = JSEvaluateScript(contextJSRef, scriptJSStringRef, globalObjectJSRef, NULL, 1, &errorJSRef);
    JSStringRelease(scriptJSStringRef);

    if(argsString.length > 300) {
      CFTimeInterval after = CACurrentMediaTime();
      CFTimeInterval elapsed = after - before;

      lowInterval = MIN(lowInterval, elapsed);
      totalInterval += elapsed;
      highInterval = MAX(highInterval, elapsed);

      totalIntervalCount++;

      printf("%f, %f, %f, %f\n", lowInterval, totalInterval, highInterval, elapsed);
    }
Contributor

robertjpayne commented Apr 27, 2015

@vjeux here's the code I have in place now, trying to formulate how I could make this more predictable / comparable but the nature of the bridge is hard.

    static CFTimeInterval lowInterval = CGFLOAT_MAX;
    static CFTimeInterval totalInterval = 0;
    static NSUInteger totalIntervalCount = 0;
    static CFTimeInterval highInterval = CGFLOAT_MIN;

    CFTimeInterval before = CACurrentMediaTime();

    JSValueRef errorJSRef = NULL;
    JSValueRef resultJSRef = NULL;
    JSGlobalContextRef contextJSRef = JSContextGetGlobalContext(strongSelf->_context.ctx);
    JSObjectRef globalObjectJSRef = JSContextGetGlobalObject(strongSelf->_context.ctx);

#pragma mark - C API

//    NSError *error;
//    NSString *argsString = (arguments.count == 1) ? RCTJSONStringify(arguments[0], &error) : RCTJSONStringify(arguments, &error);
//    if (!argsString) {
//      RCTLogError(@"Cannot convert argument to string: %@", error);
//      onComplete(nil, error);
//      return;
//    }
//    
//    // get require
//    JSStringRef requireNameJSStringRef = JSStringCreateWithUTF8CString("require");
//    JSValueRef requireJSRef = JSObjectGetProperty(contextJSRef, globalObjectJSRef, requireNameJSStringRef, &errorJSRef);
//    JSStringRelease(requireNameJSStringRef);
//    
//    if (requireJSRef != NULL && errorJSRef == NULL) {
//      
//      // get module
//      JSStringRef moduleNameJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)name);
//      JSValueRef moduleNameJSRef = JSValueMakeString(contextJSRef, moduleNameJSStringRef);
//      JSValueRef moduleJSRef = JSObjectCallAsFunction(contextJSRef, (JSObjectRef)requireJSRef, NULL, 1, (const JSValueRef *)&moduleNameJSRef, &errorJSRef);
//      JSStringRelease(moduleNameJSStringRef);
//      
//      if (moduleJSRef != NULL && errorJSRef == NULL) {
//        
//        // get method
//        JSStringRef methodNameJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)method);
//        JSValueRef methodJSRef = JSObjectGetProperty(contextJSRef, (JSObjectRef)moduleJSRef, methodNameJSStringRef, &errorJSRef);
//        JSStringRelease(methodNameJSStringRef);
//        
//        if (methodJSRef != NULL && errorJSRef == NULL) {
//          
//          // direct method invoke with no arguments
//          if (arguments.count == 0) {
//            resultJSRef = JSObjectCallAsFunction(contextJSRef, (JSObjectRef)methodJSRef, (JSObjectRef)moduleJSRef, 0, NULL, &errorJSRef);
//          }
//          // direct method invoke with 1 argument
//          else if(arguments.count == 1) {
//            JSStringRef argsJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)argsString);
//            JSValueRef argsJSRef = JSValueMakeFromJSONString(contextJSRef, argsJSStringRef);
//            resultJSRef = JSObjectCallAsFunction(contextJSRef, (JSObjectRef)methodJSRef, (JSObjectRef)moduleJSRef, 1, &argsJSRef, &errorJSRef);
//            JSStringRelease(argsJSStringRef);
//          }
//          // apply invoke with array of arguments
//          else {
//            // get apply
//            JSStringRef applyNameJSStringRef = JSStringCreateWithUTF8CString("apply");
//            JSValueRef applyJSRef = JSObjectGetProperty(contextJSRef, (JSObjectRef)methodJSRef, applyNameJSStringRef, &errorJSRef);
//            JSStringRelease(applyNameJSStringRef);
//            
//            if (applyJSRef != NULL && errorJSRef == NULL) {
//              // invoke apply
//              JSStringRef argsJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)argsString);
//              JSValueRef argsJSRef = JSValueMakeFromJSONString(contextJSRef, argsJSStringRef);
//              
//              JSValueRef args[2];
//              args[0] = JSValueMakeNull(contextJSRef);
//              args[1] = argsJSRef;
//              
//              resultJSRef = JSObjectCallAsFunction(contextJSRef, (JSObjectRef)applyJSRef, (JSObjectRef)moduleJSRef, 2, args, &errorJSRef);
//              JSStringRelease(argsJSStringRef);
//            }
//          }
//        }
//      }
//      
//    }

#pragma mark - EVALUATE SCRIPT

      NSError *error;
      NSString *argsString = RCTJSONStringify(arguments, &error);
      if (!argsString) {
        RCTLogError(@"Cannot convert argument to string: %@", error);
        onComplete(nil, error);
        return;
      }

    NSString *script = [NSString stringWithFormat:@"require('%@').%@.apply(null, %@);", name, method, argsString];
    JSStringRef scriptJSStringRef = JSStringCreateWithCFString((__bridge CFStringRef)script);
    resultJSRef = JSEvaluateScript(contextJSRef, scriptJSStringRef, globalObjectJSRef, NULL, 1, &errorJSRef);
    JSStringRelease(scriptJSStringRef);

    if(argsString.length > 300) {
      CFTimeInterval after = CACurrentMediaTime();
      CFTimeInterval elapsed = after - before;

      lowInterval = MIN(lowInterval, elapsed);
      totalInterval += elapsed;
      highInterval = MAX(highInterval, elapsed);

      totalIntervalCount++;

      printf("%f, %f, %f, %f\n", lowInterval, totalInterval, highInterval, elapsed);
    }
@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Apr 27, 2015

Contributor

cc @bryceredd who has been looking at performance measurement lately

Contributor

vjeux commented Apr 27, 2015

cc @bryceredd who has been looking at performance measurement lately

@ide

This comment has been minimized.

Show comment
Hide comment
@ide

ide Apr 27, 2015

Collaborator

As for the Objective-C API, it would presumably work just fine, but it'll be less performant due to how JSC internally maps objects and stuff.

Keeping the NSObject -> JSON -> JSValue marshalling, the Obj-C API would perform the same work as the C API plus Obj-C's message passing. I think it's a good tradeoff, given how much more easily the code reads.

Collaborator

ide commented Apr 27, 2015

As for the Objective-C API, it would presumably work just fine, but it'll be less performant due to how JSC internally maps objects and stuff.

Keeping the NSObject -> JSON -> JSValue marshalling, the Obj-C API would perform the same work as the C API plus Obj-C's message passing. I think it's a good tradeoff, given how much more easily the code reads.

@robertjpayne

This comment has been minimized.

Show comment
Hide comment
@robertjpayne

robertjpayne Apr 27, 2015

Contributor

@ide hmm really need to get some proper profiling in place. I implemented the Objective-C API locally ( though managing errors is a bit unfortunate as it's a global catch all rather than a per-function catch ) and the performance is worse than JSEvaluateScript.

The internals of JSC do a lot of weak ref and value mapping for JSValue ontop of the raw C-api.

I'm going to try and get something setup that can properly profile communication over the bridge at different payload sizes.

Contributor

robertjpayne commented Apr 27, 2015

@ide hmm really need to get some proper profiling in place. I implemented the Objective-C API locally ( though managing errors is a bit unfortunate as it's a global catch all rather than a per-function catch ) and the performance is worse than JSEvaluateScript.

The internals of JSC do a lot of weak ref and value mapping for JSValue ontop of the raw C-api.

I'm going to try and get something setup that can properly profile communication over the bridge at different payload sizes.

+ args[0] = JSValueMakeNull(contextJSRef);
+ args[1] = argsJSRef;
+
+ resultJSRef = JSObjectCallAsFunction(contextJSRef, (JSObjectRef)applyJSRef, (JSObjectRef)moduleJSRef, 2, args, &errorJSRef);

This comment has been minimized.

@tadeuzagallo

tadeuzagallo May 6, 2015

Contributor

Here it should be methodJSRef instead of moduleJSRef, since the context of apply is the actual function, not the module.

@tadeuzagallo

tadeuzagallo May 6, 2015

Contributor

Here it should be methodJSRef instead of moduleJSRef, since the context of apply is the actual function, not the module.

@tadeuzagallo

This comment has been minimized.

Show comment
Hide comment
@tadeuzagallo

tadeuzagallo May 6, 2015

Contributor

Hey, I'm merging it internally, I wrote some micro benchmarks, and it seems pretty faster, thanks! :D

I wrote a test case for that, the code won't be on the open source repo yet, but I create a gist (https://gist.github.com/tadeuzagallo/04ee3ac7017cc4e1bc8e) so you can have a look and share what you think.

I tested with a minimal amount of data, because it will have to be stringified and passed across the bridge anyway, so I don't think it should matter. Also, garbage collection make the tests a lot noisier.

The numbers are slightly volatile, but I took some time to try to make them more predictable, the tests still vary a little bit, but every test runs 400k times, and I ran them a lot of times, and the following results had a variation of less than 5%.

I just made 1 comment, but I already fixed it internally, and also had to update RCTJSONStringify to accept fragments to make the tests run.

Here are the results I got:

Previous: 20903.009869, New: 11474.709258 -> 0x1.6695ac8f62c14p+13
You made JS calls with 0 argument(s) 45.10% faster :) - Remember to update the tests with the new value: 0x1.6695ac8f62c14p+13
Previous: 22317.600661, New: 13314.305765 -> 0x1.a0127234e8a24p+13
You made JS calls with 1 argument(s) 40.34% faster :) - Remember to update the tests with the new value: 0x1.a0127234e8a24p+13
Previous: 22095.529905, New: 14783.073747 -> 0x1.cdf897088a593p+13
You made JS calls with 2 argument(s) 33.09% faster :) - Remember to update the tests with the new value: 0x1.cdf897088a593p+13
Previous: 22359.833559, New: 15148.703574 -> 0x1.d965a0eb9bf81p+13
You made JS calls with 3 argument(s) 32.25% faster :) - Remember to update the tests with the new value: 0x1.d965a0eb9bf81p+13
Contributor

tadeuzagallo commented May 6, 2015

Hey, I'm merging it internally, I wrote some micro benchmarks, and it seems pretty faster, thanks! :D

I wrote a test case for that, the code won't be on the open source repo yet, but I create a gist (https://gist.github.com/tadeuzagallo/04ee3ac7017cc4e1bc8e) so you can have a look and share what you think.

I tested with a minimal amount of data, because it will have to be stringified and passed across the bridge anyway, so I don't think it should matter. Also, garbage collection make the tests a lot noisier.

The numbers are slightly volatile, but I took some time to try to make them more predictable, the tests still vary a little bit, but every test runs 400k times, and I ran them a lot of times, and the following results had a variation of less than 5%.

I just made 1 comment, but I already fixed it internally, and also had to update RCTJSONStringify to accept fragments to make the tests run.

Here are the results I got:

Previous: 20903.009869, New: 11474.709258 -> 0x1.6695ac8f62c14p+13
You made JS calls with 0 argument(s) 45.10% faster :) - Remember to update the tests with the new value: 0x1.6695ac8f62c14p+13
Previous: 22317.600661, New: 13314.305765 -> 0x1.a0127234e8a24p+13
You made JS calls with 1 argument(s) 40.34% faster :) - Remember to update the tests with the new value: 0x1.a0127234e8a24p+13
Previous: 22095.529905, New: 14783.073747 -> 0x1.cdf897088a593p+13
You made JS calls with 2 argument(s) 33.09% faster :) - Remember to update the tests with the new value: 0x1.cdf897088a593p+13
Previous: 22359.833559, New: 15148.703574 -> 0x1.d965a0eb9bf81p+13
You made JS calls with 3 argument(s) 32.25% faster :) - Remember to update the tests with the new value: 0x1.d965a0eb9bf81p+13
@robertjpayne

This comment has been minimized.

Show comment
Hide comment
@robertjpayne

robertjpayne May 6, 2015

Contributor

@tadeuzagallo that looks awesome, so your tests indicating a 32-45% faster rate?

Thanks for the fix ups too, I'm not a JS dev so the module vs method context was a bit of naivety there.

Contributor

robertjpayne commented May 6, 2015

@tadeuzagallo that looks awesome, so your tests indicating a 32-45% faster rate?

Thanks for the fix ups too, I'm not a JS dev so the module vs method context was a bit of naivety there.

@tadeuzagallo

This comment has been minimized.

Show comment
Hide comment
@tadeuzagallo

tadeuzagallo May 6, 2015

Contributor

That's right, and practically it's closer to ~40%, since 99% of the calls are batched, and just have one argument (the array of calls to be executed).
The values of the result are in nanoseconds, so it'll be ~9 microseconds / call.

Contributor

tadeuzagallo commented May 6, 2015

That's right, and practically it's closer to ~40%, since 99% of the calls are batched, and just have one argument (the array of calls to be executed).
The values of the result are in nanoseconds, so it'll be ~9 microseconds / call.

@robertjpayne

This comment has been minimized.

Show comment
Hide comment
@robertjpayne

robertjpayne May 6, 2015

Contributor

That's still awesome though 👍 9 microseconds adds up quite quickly at 60fps! Thanks again for profiling this properly.

Contributor

robertjpayne commented May 6, 2015

That's still awesome though 👍 9 microseconds adds up quite quickly at 60fps! Thanks again for profiling this properly.

@tadeuzagallo

This comment has been minimized.

Show comment
Hide comment
@tadeuzagallo

tadeuzagallo May 6, 2015

Contributor

Sure, it was a lot of fun! Hehe
Thanks for doing that, I didn't mean to diminish it at all, it's great! Sorry if it sounded like that... I was just mentioning, because I noticed I dropped a lot of context-less values.

Contributor

tadeuzagallo commented May 6, 2015

Sure, it was a lot of fun! Hehe
Thanks for doing that, I didn't mean to diminish it at all, it's great! Sorry if it sounded like that... I was just mentioning, because I noticed I dropped a lot of context-less values.

@brentvatne

This comment has been minimized.

Show comment
Hide comment
@brentvatne

brentvatne May 6, 2015

Collaborator

Wow great job folks

Collaborator

brentvatne commented May 6, 2015

Wow great job folks

@vjeux vjeux closed this May 15, 2015

a2 pushed a commit to a2/react-native that referenced this pull request May 24, 2015

Use JSC C API for better invocation speed
Summary:
This converts the existing JSEvaluateScript call for `require('<ModuleName>').<MethodName>.apply(null, <args>);` to native JSC C API methods which shaves off about 10-15% of invocation time on average, I used pretty primitive profiling methods to track the minimum, maximum and average invocation time so would appreciate any extra eyes on the performance.

If the argument count is zero the method is invoked directly with no arguments, if the argument count is 1 it's invoked directly with just that argument. If there is more than 1 argument then apply is used and the arguments are passed as the second parameter.

Ensured all existing tests pass and instruments leaks shows nothing is leaking.
Closes facebook#1037
Github Author: Robert Payne <robertpayne@me.com>

Test Plan: Imported from GitHub, without a `Test Plan:` line.

tadeuzagallo added a commit to tadeuzagallo/react-native that referenced this pull request May 28, 2015

Use JSC C API for better invocation speed
Summary:
This converts the existing JSEvaluateScript call for `require('<ModuleName>').<MethodName>.apply(null, <args>);` to native JSC C API methods which shaves off about 10-15% of invocation time on average, I used pretty primitive profiling methods to track the minimum, maximum and average invocation time so would appreciate any extra eyes on the performance.

If the argument count is zero the method is invoked directly with no arguments, if the argument count is 1 it's invoked directly with just that argument. If there is more than 1 argument then apply is used and the arguments are passed as the second parameter.

Ensured all existing tests pass and instruments leaks shows nothing is leaking.
Closes facebook#1037
Github Author: Robert Payne <robertpayne@me.com>

Test Plan: Imported from GitHub, without a `Test Plan:` line.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment