Performance enhancement by avoiding conversion

Discuss the JavaScriptCore Objective-C wrapper/bridge
Post Reply
ddribin
Posts: 19
Joined: Mon May 12, 2008 9:21 am

Performance enhancement by avoiding conversion

Post by ddribin » Fri May 30, 2008 7:45 pm

Okay, I think this is the last performance enhancement I've got. :) Here's the patch, made against svn r7 for another 12% speedup in certain circumstances:

http://www.dribin.org/dave/tmp/jskit-r7 ... alue.patch

It does two things. First, instead of converting the JSValueRef * arguments in invokeWithContext: into an NSArray, I created an NSArray subclass: JSKitArgumentArray. It overrides objectAtIndex: and does the conversion to an id there. This means, it only does the conversion from JSValueRef to id on demand. Handy for variable length arguments, but it also avoids calling NSMutableArray addObject:. I also added this method:

Code: Select all

- (JSValueRef)jsvalueAtIndex:(NSUInteger)index;
This also means, an ObjC method can grab the JSValueRef directly, instead of converting to an id. For primitive types like JavaScript numbers, this avoids the conversion to NSNumber.

The second change of this patch allows an ObjC method to return a JSValueRef instead of an id. Again, this means if a JavaScript number is returned, we can avoid the overhead of wrapping it in an NSNumber.

Best of all, this patch is completely backwards compatible. How? Because JSKitArgumentArray is a subclass of NSArray, all existing functions added to the interpreter still work. Also, I added this method to JSKitInterpreter:

Code: Select all

- (void) addFunction: (NSString *) name target: (id) target selector: (SEL) sel
  convertReturnValue: (BOOL) convertReturnValue;
If convertReturnValue is YES, then the ObjC function is expected to return an id, just as before, and it is converted to JSValueRef. The original addFunction: method calls this new method with YES. If convertReturnValue is NO, then the ObjC function is expected to return a JSValueRef. In essence, the signature is:

Code: Select all

- (JSValueRef)objcFunction:(JSKitArgumentArray *)arguments;
Since this is a new method, it is essentially an "opt-in" optimization.

In one of my benchmarks (which evaluates a script 5,000 times), this patch increases performance from 12.6 seconds to 11.1 seconds, i.e. a 12% speedup. Of course, this benchmark has a function that uses JavaScript numbers, so this speedup won't be universal.

-Dave

gandreas
Immortal
Posts: 1464
Joined: Wed Feb 04, 2004 6:02 pm
Contact:

Post by gandreas » Sun Jun 01, 2008 10:47 am

I like the idea of encapsulating the JavaScript parameters, though I'm going to have to give it some more thought. At the very least there are problems with the fact that the JS parameters are stack based, and the object encapsulating them could be be retained and live on past their existence (for example, some sort of bizarre Undo mechanism, or in some sort of future debugging enhancement). So the subclass should at least make its own copies of the parameters and make sure that they are retained in the JS world. Of course, that'll slow it down somewhat, but make it safe.

The "return a JSValueRef" enhancement is more curious. The problem is that I want to avoid having the embedding app needed to know anything about JS internals (if for no other reason than the fact that it prevents building on 10.4 using SDKs). And so while there are "void *" definitions for the various JSValueRef, etc... the embedding app doesn't actually see any of the JavaScriptCore interfaces to be able to manipulate these.

So perhaps the better solution is to combine both of these features, and provide a different method signature for the callback that both expects and returns raw JSValueRef's.

Then, the signature for the callback would be more like

Code: Select all

- (JSValueRef) objCFunctionWithContext:(JSContextRef)context
        argumentCount:(size_t)argumentCount
            arguments:(const JSValueRef *)arguments
The addFunction method would be unchanged, it would just add logic to check what kind of selector was passed (perhaps checking the NSMethodSignature information).

It would be the "in for a penny, in for a pound" optimization (you either get the completely transparent, but slower, version, or the fast as possible, but you need to handle all the dirty details, version).

gandreas
Immortal
Posts: 1464
Joined: Wed Feb 04, 2004 6:02 pm
Contact:

Post by gandreas » Tue Jun 03, 2008 2:27 pm

OK, I've added a "raw fast path" (that should be even faster than what you've got because it adds a little IMP caching). So basically, instead of using

Code: Select all

[interpreter addFunction: @"Foo" target: self selector: @selector(doSomethingWithParameterArray:)];
...
- (id) doSomethingWithParameterArray: (NSArray *) parameters
one instead does

Code: Select all

[interpreter addRawFunction: @"Foo" target: self selector: @selector(doSomethingWithContext:arguments:exception:)];
...
- (JSValueRef) doSomethingWithContex: (JSContextRef) ctx arguments: (const JSValueRef *)arguments count: (size_t) count exception: (JSValueRef*) exception
Basically, one deals with the "raw" JavaScriptCore APIs and data types. Obviously, this isn't for everybody (especially since it will require dealing with the APIs that aren't going to be easy to get to if you compile against a 10.4u SDK), but it should be faster.

ddribin
Posts: 19
Joined: Mon May 12, 2008 9:21 am

Post by ddribin » Wed Jun 04, 2008 6:01 pm

I upgraded to r9 to try this out. I found a couple of bugs. Here's a patch:

http://dl.getdropbox.com/u/27036/tmp/JSKit-raw-r9.patch

The numberOfArguments check was incorrect (should be 6 args, not 5) and it was retaining target when it shouldn't be.

Other than that, it works great! Thanks! Not sure the IMP caching is worth it, as its not really any faster than my version, but this is definitely speedy. :) Can't wait to try out SquirrelFish.

-Dave

ddribin
Posts: 19
Joined: Mon May 12, 2008 9:21 am

Post by ddribin » Mon Jul 28, 2008 10:45 pm

Hello,

Do you plan on applying that patch? The current code in Subversion is broken in regards to this.

-Dave

gandreas
Immortal
Posts: 1464
Joined: Wed Feb 04, 2004 6:02 pm
Contact:

Post by gandreas » Wed Jul 30, 2008 10:18 am

Yes, I had an update ready to be checked in, but some svn problems prevented that (and then along came iPhone projects, etc...)

I'm hoping to have a real "packaged" semi-stable release at some point soon, but a potentially serious issue has come up that can interfere with that (hint - involves lawyers).

Post Reply

Who is online

Users browsing this forum: No registered users and 1 guest