From c835641dcbc38b2e88b7e24cc6ba44bd6a05e7bd Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 20 Mar 2014 22:36:43 -0400 Subject: [PATCH] Private methods are no longer wrapped This is an exciting performance optimization that seems to have eluded me for a surprisingly long time, given that the realization was quite random. ease.js accomplishes much of its work through a method wrapper---each and every method definition (well, until now) was wrapped in a closure that performed a number of steps, depending on the type of wrapper involved: 1. All wrappers perform a context lookup, binding to the instance's private member object of the class that defined that particular method. (See "Implementation Details" in the manual for more information.) 2. This context is restored upon returning from the call: if a method returns `this', it is instead converted back to the context in which the method was invoked, which prevents the private member object from leaking out of a public interface. 3. In the event of an override, this.__super is set up (and torn down). There are other details (e.g. the method wrapper used for method proxies), but for the sake of this particular commit, those are the only ones that really matter. There are a couple of important details to notice: - Private members are only ever accessible from within the context of the private member object, which is always the context when executing a method. - Private methods cannot be overridden, as they cannot be inherited. Consequently: 1. We do not need to perform a context lookup: we are already in the proper context. 2. We do not need to restore the context, as we never needed to change it to begin with. 3. this.__super is never applicable. Method wrappers are therefore never necessary for private methods; they have therefore been removed. This has some interesting performance implications. While in most cases the overhead of method wrapping is not a bottleneck, it can have a strong impact in the event of frequent method calls or heavily recursive algorithms. There was one particular problem that ease.js suffered from, which is mentioned in the manual: recursive calls to methods in ease.js were not recommended because it (a) made two function calls for each method call, effectively halving the remaining call stack size, and (b) tail call optimization could not be performed, because recursion invoked the wrapper, *not* the function that was wrapped. By removing the method wrapper on private methods, we solve both of these problems; now, heavily recursive algorithms need only use private methods (which could always be exposed through a protected or public API) when recursing to entirely avoid any performance penalty by using ease.js. Running the test cases on my system (your results may vary) before and after the patch, we have: BEFORE: 0.170s (x1000 = 0.0001700000s each): Declare 1000 anonymous classes with private members 0.021s (x500000 = 0.0000000420s each): Invoke private methods internally AFTER: 0.151s (x1000 = 0.0001510000s each): Declare 1000 anonymous classes with private members 0.004s (x500000 = 0.0000000080s each): Invoke private methods internally This is all the more motivation to use private members, which enforces encapsulation; keep in mind that, because use of private members is the ideal in well-encapsulated and well-factored code, ease.js has been designed to perform best under those circumstances. --- lib/MemberBuilder.js | 7 +++-- test/MemberBuilder/MethodTest.js | 47 +++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/lib/MemberBuilder.js b/lib/MemberBuilder.js index c2419be..680cef8 100644 --- a/lib/MemberBuilder.js +++ b/lib/MemberBuilder.js @@ -173,9 +173,12 @@ exports.buildMethod = function( } } - else if ( keywords[ 'abstract' ] ) + else if ( keywords[ 'abstract' ] || keywords[ 'private' ] ) { - // we do not want to wrap abstract methods, since they are not callable + // we do not want to wrap abstract methods, since they are not + // callable; further, we do not need to wrap private methods, since + // they are only ever accessible when we are already within a + // private context (see test case for more information) dest[ name ] = value; } else diff --git a/test/MemberBuilder/MethodTest.js b/test/MemberBuilder/MethodTest.js index d2aa8ee..82bc337 100644 --- a/test/MemberBuilder/MethodTest.js +++ b/test/MemberBuilder/MethodTest.js @@ -67,7 +67,11 @@ require( 'common' ).testCase( // stub factories used for testing var stubFactory = this.require( 'MethodWrapperFactory' )( - function( func ) { return func; } + function( func ) + { + // still wrap it so that the function is encapsulated + return function() { return func() }; + } ); // used for testing proxies explicitly @@ -282,4 +286,45 @@ require( 'common' ).testCase( this.members[ 'public' ].foo(); this.assertOk( called, "Override unkept" ); }, + + + /** + * This is a beautiful consequence of the necessay context in which + * private methods must be invoked. + * + * If a method has the ability to call a private method, then we must + * already be within a private context (that is---using the private + * member object, which happens whenever we're executing a method of + * that class). The purpose of the method wrapper is to (a) determine + * the proper context, (b) set up super method references, and (c) + * restore the context in the event that the method returns `this'. Not + * a single one of these applies: (a) is void beacuse we are already in + * the proper context; (b) is not applicable since private methods + * cannot have a super method; and (c) we do not need to restore context + * before returning because the context would be the same (per (a)). + * + * That has excellent performance implications: not only do we reduce + * class building times for private methods, but we also improve method + * invocation times, since we do not have to invoke a *closure* for each + * and every method call. Further, recursive private methods are no + * longer an issue since they do not gobble up the stack faster and, + * consequently, the JavaScript engine can now take advantage of tail + * call optimizations. + * + * This is also further encouragement to use private members. :) + */ + 'Private methods are not wrapped': function() + { + var f = function() {}, + name = 'foo'; + + this.sut.buildMethod( + this.members, {}, name, f, { 'private': true }, + function() {}, 1, {} + ); + + // if the private method was not wrapped, then it should have been + // assigned to the member object unencapsulated + this.assertStrictEqual( this.members[ 'private' ][ name ], f ); + }, } );