From 37bf2fa353e8922d7a663c8f9c061d3192be949a Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 10 Mar 2011 22:43:36 -0500 Subject: [PATCH 01/17] Began adding performance tests --- test/perf/README | 8 +++ test/perf/common.js | 91 ++++++++++++++++++++++++ test/perf/perf-class-define.js | 42 +++++++++++ test/perf/perf-class-inst-anon-empty.js | 43 +++++++++++ test/perf/perf-class-inst-named-empty.js | 44 ++++++++++++ test/perf/perf-class-require.js | 33 +++++++++ 6 files changed, 261 insertions(+) create mode 100644 test/perf/README create mode 100644 test/perf/common.js create mode 100644 test/perf/perf-class-define.js create mode 100644 test/perf/perf-class-inst-anon-empty.js create mode 100644 test/perf/perf-class-inst-named-empty.js create mode 100644 test/perf/perf-class-require.js diff --git a/test/perf/README b/test/perf/README new file mode 100644 index 0000000..9c95d99 --- /dev/null +++ b/test/perf/README @@ -0,0 +1,8 @@ +This directory contains the performance tests. These tests contain basic +routines that perform a single action and output the result in seconds, with a +basic description of what has been done. The timing is done via the native Date +object to ensure that it can be run both server and client-side. + +It is important that each test performs only a single operation to ensure that +the prior operations have no consequences on the previous, at least when run +server-side by invoking a separate executable for each. diff --git a/test/perf/common.js b/test/perf/common.js new file mode 100644 index 0000000..75ec0cf --- /dev/null +++ b/test/perf/common.js @@ -0,0 +1,91 @@ +/** + * Common performance testing functionality + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + +/** + * Stores start time + * @type {number} + */ +var start = 0; + + +/** + * Includes a module from the lib directory + * + * @param {string} name module name + * + * @return {Object} module exports + */ +exports.require = function( name ) +{ + return require( '../../lib/' + name ); +}; + + +/** + * A simple wrapper to perform testing and output the result + * + * @param {function()} test performance test to perform + * @param {string=} desc test description + * + * @return {undefined} + */ +exports.test = function( test, desc ) +{ + exports.start(); + test(); + exports.report( desc ); +}; + + +/** + * Starts the timer + * + * @return {undefined} + */ +exports.start = function() +{ + start = ( new Date() ).getTime(); +}; + + +/** + * Outputs the time elapsed, followed by the description (if available) + * + * @param {string=} desc test description + * + * @return {undefined} + */ +exports.report = function( desc ) +{ + desc = desc || ''; + + var end = ( new Date() ).getTime(), + total = ( ( end - start ) / 1000 ).toFixed( 3 ) + ; + + console.log( total + 's' + + ( ( desc ) ? ( ': ' + desc ) : '' ) + ); +}; + diff --git a/test/perf/perf-class-define.js b/test/perf/perf-class-define.js new file mode 100644 index 0000000..b4eb20b --- /dev/null +++ b/test/perf/perf-class-define.js @@ -0,0 +1,42 @@ +/** + * Tests amount of time taken to declare 1000 classes + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( {} ); + } + +}, 'Declare ' + count + ' empty anonymous classes' ); diff --git a/test/perf/perf-class-inst-anon-empty.js b/test/perf/perf-class-inst-anon-empty.js new file mode 100644 index 0000000..69220a8 --- /dev/null +++ b/test/perf/perf-class-inst-anon-empty.js @@ -0,0 +1,43 @@ +/** + * Tests amount of time taken to instantiate anonymous classes + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 5000, + Foo = Class( {} ) +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Foo(); + } + +}, 'Instantiate ' + count + ' empty anonymous classes' ); diff --git a/test/perf/perf-class-inst-named-empty.js b/test/perf/perf-class-inst-named-empty.js new file mode 100644 index 0000000..5ee920f --- /dev/null +++ b/test/perf/perf-class-inst-named-empty.js @@ -0,0 +1,44 @@ +/** + * Tests amount of time taken to instantiate named classes + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 5000, + Foo = Class( 'Foo', {} ) +; + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + // to be extra confident that V8 or another compiler won't realize this + // is useless and optimize it out + Foo(); + } + +}, 'Instantiate ' + count + ' empty named classes' ); diff --git a/test/perf/perf-class-require.js b/test/perf/perf-class-require.js new file mode 100644 index 0000000..065c50d --- /dev/null +++ b/test/perf/perf-class-require.js @@ -0,0 +1,33 @@ +/** + * Tests amount of time spent on requiring class module + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ); + +// we run this test once because require() will cache the object in memory +common.test( function() +{ + common.require( 'class' ); +}, 'Require class module' ); + From 689a67405b4e49bb865258246ca4f2cbf1a96f4f Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 10 Mar 2011 22:55:07 -0500 Subject: [PATCH 02/17] Added performance tests to Makefile --- Makefile | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index a277102..8e84c96 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,8 @@ PATH_TOOLS=./tools PATH_COMBINE_OUTPUT=${PATH_BUILD}/ease.js PATH_COMBINE_OUTPUT_FULL=${PATH_BUILD}/ease-full.js PATH_BROWSER_TEST=${PATH_TOOLS}/browser-test.html +PATH_TEST=./test +PATH_PERF_TEST=${PATH_TEST}/perf COMBINE=${PATH_TOOLS}/combine @@ -29,12 +31,15 @@ test: default for test in `find ./test -name 'test-*.js'`; do \ node $${test}; \ done; \ - for test in `find ./test -regex '.*/test-[^\.]*'`; do \ ./$$test; \ done; +# performance tests +perf: default + find "${PATH_PERF_TEST}" -name 'perf-*.js' -exec node {} \; + # clean up build dir clean: rm -rf ${PATH_BUILD} - + From cdc88bea4cdec758e3eff4cefdeb7fabbaf22cf0 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 10 Mar 2011 23:07:52 -0500 Subject: [PATCH 03/17] Added count to perf output to calculate individual cost --- test/perf/common.js | 26 ++++++++++++++++-------- test/perf/perf-class-define.js | 2 +- test/perf/perf-class-inst-anon-empty.js | 2 +- test/perf/perf-class-inst-named-empty.js | 2 +- test/perf/perf-class-require.js | 2 +- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/test/perf/common.js b/test/perf/common.js index 75ec0cf..dc69cc4 100644 --- a/test/perf/common.js +++ b/test/perf/common.js @@ -45,16 +45,21 @@ exports.require = function( name ) /** * A simple wrapper to perform testing and output the result * - * @param {function()} test performance test to perform - * @param {string=} desc test description + * The count is not used to call the function multiple times, because that would + * greatly impact the test results. Instead, you should pass the number of times + * the test was performed in a loop. + * + * @param {function()} test performance test to perform + * @param {number} count number of times the test was performed + * @param {string=} desc test description * * @return {undefined} */ -exports.test = function( test, desc ) +exports.test = function( test, count, desc ) { exports.start(); test(); - exports.report( desc ); + exports.report( count, desc ); }; @@ -72,19 +77,22 @@ exports.start = function() /** * Outputs the time elapsed, followed by the description (if available) * - * @param {string=} desc test description + * @param {number} count number of times the test was performed + * @param {string=} desc test description * * @return {undefined} */ -exports.report = function( desc ) +exports.report = function( count, desc ) { - desc = desc || ''; + count = +count; + desc = desc || ''; var end = ( new Date() ).getTime(), - total = ( ( end - start ) / 1000 ).toFixed( 3 ) + total = ( ( end - start ) / 1000 ).toFixed( 3 ), + pers = ( total / count ).toFixed( 7 ) ; - console.log( total + 's' + + console.log( total + "s (x" + count + " = " + pers + "s each)" + ( ( desc ) ? ( ': ' + desc ) : '' ) ); }; diff --git a/test/perf/perf-class-define.js b/test/perf/perf-class-define.js index b4eb20b..ad448db 100644 --- a/test/perf/perf-class-define.js +++ b/test/perf/perf-class-define.js @@ -39,4 +39,4 @@ common.test( function() Class( {} ); } -}, 'Declare ' + count + ' empty anonymous classes' ); +}, count, 'Declare ' + count + ' empty anonymous classes' ); diff --git a/test/perf/perf-class-inst-anon-empty.js b/test/perf/perf-class-inst-anon-empty.js index 69220a8..d737f95 100644 --- a/test/perf/perf-class-inst-anon-empty.js +++ b/test/perf/perf-class-inst-anon-empty.js @@ -40,4 +40,4 @@ common.test( function() Foo(); } -}, 'Instantiate ' + count + ' empty anonymous classes' ); +}, count, 'Instantiate ' + count + ' empty anonymous classes' ); diff --git a/test/perf/perf-class-inst-named-empty.js b/test/perf/perf-class-inst-named-empty.js index 5ee920f..1149909 100644 --- a/test/perf/perf-class-inst-named-empty.js +++ b/test/perf/perf-class-inst-named-empty.js @@ -41,4 +41,4 @@ common.test( function() Foo(); } -}, 'Instantiate ' + count + ' empty named classes' ); +}, count, 'Instantiate ' + count + ' empty named classes' ); diff --git a/test/perf/perf-class-require.js b/test/perf/perf-class-require.js index 065c50d..c5bd893 100644 --- a/test/perf/perf-class-require.js +++ b/test/perf/perf-class-require.js @@ -29,5 +29,5 @@ var common = require( __dirname + '/common.js' ); common.test( function() { common.require( 'class' ); -}, 'Require class module' ); +}, 1, 'Require class module' ); From a8767a1da0430b35359047b8a2eb8215777fedaf Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 10 Mar 2011 23:32:33 -0500 Subject: [PATCH 04/17] Added property/method class definition performance tests --- ...rf-class-define-methods-keyword-private.js | 47 +++++++++++++++++++ ...-class-define-methods-keyword-protected.js | 47 +++++++++++++++++++ ...erf-class-define-methods-keyword-public.js | 47 +++++++++++++++++++ test/perf/perf-class-define-methods.js | 47 +++++++++++++++++++ test/perf/perf-class-define-named.js | 42 +++++++++++++++++ ...class-define-properties-keyword-private.js | 47 +++++++++++++++++++ ...ass-define-properties-keyword-protected.js | 47 +++++++++++++++++++ ...-class-define-properties-keyword-public.js | 47 +++++++++++++++++++ test/perf/perf-class-define-properties.js | 47 +++++++++++++++++++ 9 files changed, 418 insertions(+) create mode 100644 test/perf/perf-class-define-methods-keyword-private.js create mode 100644 test/perf/perf-class-define-methods-keyword-protected.js create mode 100644 test/perf/perf-class-define-methods-keyword-public.js create mode 100644 test/perf/perf-class-define-methods.js create mode 100644 test/perf/perf-class-define-named.js create mode 100644 test/perf/perf-class-define-properties-keyword-private.js create mode 100644 test/perf/perf-class-define-properties-keyword-protected.js create mode 100644 test/perf/perf-class-define-properties-keyword-public.js create mode 100644 test/perf/perf-class-define-properties.js diff --git a/test/perf/perf-class-define-methods-keyword-private.js b/test/perf/perf-class-define-methods-keyword-private.js new file mode 100644 index 0000000..3c7ecce --- /dev/null +++ b/test/perf/perf-class-define-methods-keyword-private.js @@ -0,0 +1,47 @@ +/** + * Tests amount of time taken to declare 1000 classes with a few members, + * private keyword + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( { + 'private a': function() {}, + 'private b': function() {}, + 'private c': function() {}, + } ); + } + +}, count, 'Declare ' + count + ' anonymous classes with private members' ); diff --git a/test/perf/perf-class-define-methods-keyword-protected.js b/test/perf/perf-class-define-methods-keyword-protected.js new file mode 100644 index 0000000..611b8f4 --- /dev/null +++ b/test/perf/perf-class-define-methods-keyword-protected.js @@ -0,0 +1,47 @@ +/** + * Tests amount of time taken to declare 1000 classes with a few members, + * protected keyword + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( { + 'protected a': function() {}, + 'protected b': function() {}, + 'protected c': function() {}, + } ); + } + +}, count, 'Declare ' + count + ' anonymous classes with protected members' ); diff --git a/test/perf/perf-class-define-methods-keyword-public.js b/test/perf/perf-class-define-methods-keyword-public.js new file mode 100644 index 0000000..8820713 --- /dev/null +++ b/test/perf/perf-class-define-methods-keyword-public.js @@ -0,0 +1,47 @@ +/** + * Tests amount of time taken to declare 1000 classes with a few members, public + * keyword + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( { + 'public a': function() {}, + 'public b': function() {}, + 'public c': function() {}, + } ); + } + +}, count, 'Declare ' + count + ' anonymous classes with public members' ); diff --git a/test/perf/perf-class-define-methods.js b/test/perf/perf-class-define-methods.js new file mode 100644 index 0000000..f51a1c6 --- /dev/null +++ b/test/perf/perf-class-define-methods.js @@ -0,0 +1,47 @@ +/** + * Tests amount of time taken to declare 1000 classes with a few members, no + * keywords + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( { + a: function() {}, + b: function() {}, + c: function() {}, + } ); + } + +}, count, 'Declare ' + count + ' anonymous classes with few methods' ); diff --git a/test/perf/perf-class-define-named.js b/test/perf/perf-class-define-named.js new file mode 100644 index 0000000..16f52e9 --- /dev/null +++ b/test/perf/perf-class-define-named.js @@ -0,0 +1,42 @@ +/** + * Tests amount of time taken to declare 1000 classes + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( 'Foo', {} ); + } + +}, count, 'Declare ' + count + ' empty named classes' ); diff --git a/test/perf/perf-class-define-properties-keyword-private.js b/test/perf/perf-class-define-properties-keyword-private.js new file mode 100644 index 0000000..c3ed765 --- /dev/null +++ b/test/perf/perf-class-define-properties-keyword-private.js @@ -0,0 +1,47 @@ +/** + * Tests amount of time taken to declare 1000 classes with a few properties, + * private keyword + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( { + 'private a': 'foo', + 'private b': 10, + 'private c': false, + } ); + } + +}, count, 'Declare ' + count + ' anonymous classes with private properties' ); diff --git a/test/perf/perf-class-define-properties-keyword-protected.js b/test/perf/perf-class-define-properties-keyword-protected.js new file mode 100644 index 0000000..07f19a4 --- /dev/null +++ b/test/perf/perf-class-define-properties-keyword-protected.js @@ -0,0 +1,47 @@ +/** + * Tests amount of time taken to declare 1000 classes with a few properties, + * protected keyword + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( { + 'protected a': 'foo', + 'protected b': 10, + 'protected c': false, + } ); + } + +}, count, 'Declare ' + count + ' anonymous classes with protected properties' ); diff --git a/test/perf/perf-class-define-properties-keyword-public.js b/test/perf/perf-class-define-properties-keyword-public.js new file mode 100644 index 0000000..569ee5e --- /dev/null +++ b/test/perf/perf-class-define-properties-keyword-public.js @@ -0,0 +1,47 @@ +/** + * Tests amount of time taken to declare 1000 classes with a few properties, + * public keyword + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( { + 'public a': 'foo', + 'public b': 10, + 'public c': false, + } ); + } + +}, count, 'Declare ' + count + ' anonymous classes with public properties' ); diff --git a/test/perf/perf-class-define-properties.js b/test/perf/perf-class-define-properties.js new file mode 100644 index 0000000..97046d1 --- /dev/null +++ b/test/perf/perf-class-define-properties.js @@ -0,0 +1,47 @@ +/** + * Tests amount of time taken to declare 1000 classes with few properties, no + * keywords + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ) + + count = 1000 +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + Class( { + a: 'foo', + b: 10, + c: false, + } ); + } + +}, count, 'Declare ' + count + ' anonymous classes with few properties' ); From 311e39d67cb25013aa0c417e9da77a59ab822486 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 11 Mar 2011 19:14:10 -0500 Subject: [PATCH 05/17] Added performance test for reading properties, internal and external --- test/perf/common.js | 2 +- test/perf/perf-class-get-property.js | 101 +++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 test/perf/perf-class-get-property.js diff --git a/test/perf/common.js b/test/perf/common.js index dc69cc4..538d7a7 100644 --- a/test/perf/common.js +++ b/test/perf/common.js @@ -89,7 +89,7 @@ exports.report = function( count, desc ) var end = ( new Date() ).getTime(), total = ( ( end - start ) / 1000 ).toFixed( 3 ), - pers = ( total / count ).toFixed( 7 ) + pers = ( total / count ).toFixed( 10 ) ; console.log( total + "s (x" + count + " = " + pers + "s each)" + diff --git a/test/perf/perf-class-get-property.js b/test/perf/perf-class-get-property.js new file mode 100644 index 0000000..4e3a0ed --- /dev/null +++ b/test/perf/perf-class-get-property.js @@ -0,0 +1,101 @@ +/** + * Tests amount of time taken to declare read properties internally and + * externally + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ), + + // we need many tests for a measurable result + count = 500000 + + // instance of anonymous class + foo = Class( { + 'public pub_bar': 'foo', + 'protected prot_bar': 'bar', + 'private priv_bar': 'baz', + + 'public testInternal': function() + { + var _self = this; + + common.test( function() + { + var i = count, + val = null + ; + + while ( i-- ) + { + val = _self.pub_bar; + } + }, count, 'Read public properties internally' ); + + + common.test( function() + { + var i = count, + val = null + ; + + while ( i-- ) + { + val = _self.prot_bar; + } + }, count, 'Read protected properties internally' ); + + + common.test( function() + { + var i = count, + val = null + ; + + while ( i-- ) + { + val = _self.priv_bar; + } + }, count, 'Read private properties internally' ); + }, + } )() +; + + +common.test( function() +{ + var i = count, + val = null + ; + + while ( i-- ) + { + val = foo.pub_bar; + } + +}, count, 'Read public properties externally' ); + + +// run the same test internally +foo.testInternal(); + From 61d78179a398740b4089fd5aa4224c35d0c7e40d Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 11 Mar 2011 19:16:11 -0500 Subject: [PATCH 06/17] Added member redeclaration bug fix to TODO --- TODO | 1 + 1 file changed, 1 insertion(+) diff --git a/TODO b/TODO index 925d8c8..1255025 100644 --- a/TODO +++ b/TODO @@ -5,6 +5,7 @@ [ target: 0.1.0 ] Misc - Class module is becoming too large; refactor + - Disallow member redeclaration in definition Member Keywords - Restrictions; throw exceptions when unknown keywords are used From d2aa24ef6624bb4b7fa25d9995bfede4568682a0 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 11 Mar 2011 19:16:52 -0500 Subject: [PATCH 07/17] Added performance tests for setting properties, internally and externally --- test/perf/perf-class-set-property.js | 101 +++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 test/perf/perf-class-set-property.js diff --git a/test/perf/perf-class-set-property.js b/test/perf/perf-class-set-property.js new file mode 100644 index 0000000..0221d5a --- /dev/null +++ b/test/perf/perf-class-set-property.js @@ -0,0 +1,101 @@ +/** + * Tests amount of time taken to declare read properties internally and + * externally + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ), + + // we need many tests for a measurable result + count = 500000 + + // instance of anonymous class + foo = Class( { + 'public pub_bar': 'foo', + 'protected prot_bar': 'bar', + 'private priv_bar': 'baz', + + 'public testInternal': function() + { + var _self = this; + + common.test( function() + { + var i = count, + val = null + ; + + while ( i-- ) + { + _self.pub_bar = 'foo'; + } + }, count, 'Write public properties internally' ); + + + common.test( function() + { + var i = count, + val = null + ; + + while ( i-- ) + { + _self.prot_bar = 'foo'; + } + }, count, 'Write protected properties internally' ); + + + common.test( function() + { + var i = count, + val = null + ; + + while ( i-- ) + { + _self.priv_bar = 'foo'; + } + }, count, 'Write private properties internally' ); + }, + } )() +; + + +common.test( function() +{ + var i = count, + val = null + ; + + while ( i-- ) + { + foo.pub_bar = 'foo'; + } + +}, count, 'Write public properties externally' ); + + +// run the same test internally +foo.testInternal(); + From 5a420fae4e8180ee1f456cee74cdb4293e5219c5 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 11 Mar 2011 19:20:05 -0500 Subject: [PATCH 08/17] Added perf tests for invoking class methods --- test/perf/perf-class-invoke-method.js | 97 +++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 test/perf/perf-class-invoke-method.js diff --git a/test/perf/perf-class-invoke-method.js b/test/perf/perf-class-invoke-method.js new file mode 100644 index 0000000..db994ee --- /dev/null +++ b/test/perf/perf-class-invoke-method.js @@ -0,0 +1,97 @@ +/** + * Tests amount of time taken to invoke Class methods + * + * Results are assigned to a var to ensure V8 doesn't optimize the calls too + * much. + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package performance + */ + + +var common = require( __dirname + '/common.js' ), + Class = common.require( 'class' ), + + count = 500000, + + // used to ensure v8 doesn't optimize functions away + i = 0, + + // instance of anonymous class + foo = Class( { + 'public pub': function() { i++; }, + 'protected prot': function() { i++; }, + 'private priv': function() { i++; }, + + 'public testInternal': function() + { + var _self = this; + + common.test( function() + { + var i = count; + + while ( i-- ) + { + _self.pub(); + } + }, count, 'Invoke public methods internally' ); + + + common.test( function() + { + var i = count; + + while ( i-- ) + { + _self.prot(); + } + }, count, 'Invoke protected methods internally' ); + + + common.test( function() + { + var i = count; + + while ( i-- ) + { + _self.priv(); + } + }, count, 'Invoke private methods internally' ); + }, + } )() +; + + +common.test( function() +{ + var i = count; + + while ( i-- ) + { + foo.pub(); + } + +}, count, 'Invoke public methods externally' ); + + +// run the same test internally +foo.testInternal(); + From 05e249def46ebb28cbe7c1d9e91aeaa6a00902fc Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 11 Mar 2011 19:27:07 -0500 Subject: [PATCH 09/17] Added result explanations to method invocation performance tests --- test/perf/perf-class-invoke-method.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/perf/perf-class-invoke-method.js b/test/perf/perf-class-invoke-method.js index db994ee..bc8b5b8 100644 --- a/test/perf/perf-class-invoke-method.js +++ b/test/perf/perf-class-invoke-method.js @@ -1,8 +1,18 @@ /** * Tests amount of time taken to invoke Class methods * - * Results are assigned to a var to ensure V8 doesn't optimize the calls too - * much. + * The expected results are as follows: + * - Method invocations are expected to be slower than invoking a method on a + * conventional constructor instance. This is because of the method wrapper + * used by ease.js. + * - Public methods externally should be invoked very quickly. They are part + * of the class's prototype and therefore easily accessible. + * - Public methods /internally/ are likely to be invoked slightly more + * slowly. This is because it takes one extra step down the prototype chain + * to access them. The difference should be minute. + * - Protected and private methods internally should be accessed fairly + * quickly since, like public methods externally, they are first on the + * prototype chain. * * Copyright (C) 2010 Mike Gerwitz * From db5a5bac58b6676464d8d659e6a87a4248055335 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 11 Mar 2011 19:47:00 -0500 Subject: [PATCH 10/17] Reformatted Makefile to support parallel processing of performance tests --- Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 8e84c96..fe16753 100644 --- a/Makefile +++ b/Makefile @@ -7,6 +7,8 @@ PATH_BROWSER_TEST=${PATH_TOOLS}/browser-test.html PATH_TEST=./test PATH_PERF_TEST=${PATH_TEST}/perf +PERF_TESTS := $(shell find "$(PATH_PERF_TEST)" -name 'perf-*.js') + COMBINE=${PATH_TOOLS}/combine @@ -36,8 +38,9 @@ test: default done; # performance tests -perf: default - find "${PATH_PERF_TEST}" -name 'perf-*.js' -exec node {} \; +perf: default $(PERF_TESTS) +perf-%.js: default + @node $@ # clean up build dir clean: From f572e53e9ded3bdc414870f8c1692a6974ece42a Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 11 Mar 2011 19:58:24 -0500 Subject: [PATCH 11/17] Distinction between JS and shell tests unnecessary in 'find' --- Makefile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 0cf386c..8388f9b 100644 --- a/Makefile +++ b/Makefile @@ -7,8 +7,7 @@ PATH_BROWSER_TEST=${PATH_TOOLS}/browser-test.html COMBINE=${PATH_TOOLS}/combine -TESTS_JS := $(shell find "./test" -name 'test-*.js') -TESTS_SHELL := $(shell find "./test" -name 'test-[^\.]*') +TESTS := $(shell find "./test" -name 'test-*') .PHONY: test @@ -28,7 +27,7 @@ combine: mkbuild cp ${PATH_BROWSER_TEST} ${PATH_BUILD} # run tests -test: default $(TESTS_JS) $(TESTS_SHELL) +test: default $(TESTS) test-%.js: default node $@ test-%: default From 342dfd63d651e4e2310ae5578deb0ae5bf26804d Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 11 Mar 2011 20:00:04 -0500 Subject: [PATCH 12/17] Extra tab in Makefile --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 8388f9b..7dad5f1 100644 --- a/Makefile +++ b/Makefile @@ -29,9 +29,9 @@ combine: mkbuild # run tests test: default $(TESTS) test-%.js: default - node $@ + node $@ test-%: default - ./$@ + ./$@ # clean up build dir clean: From 540d8a4f000730c9da3ba91a5be2b9a490c3ab6d Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 12 Mar 2011 12:07:07 -0500 Subject: [PATCH 13/17] Altered Makefile to ensure the combine test is performed after all others --- Makefile | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index b7c3192..c8cfbaf 100644 --- a/Makefile +++ b/Makefile @@ -11,10 +11,14 @@ PERF_TESTS := $(shell find "$(PATH_PERF_TEST)" -name 'perf-*.js') COMBINE=${PATH_TOOLS}/combine -TESTS := $(shell find "./test" -name 'test-*') +TESTS := $(shell find "$(PATH_TEST)" \ + -name 'test-*' \ + -a ! -name 'test-combine.js'\ +) +TEST_COMBINE := $(PATH_TEST)/test-combine.js -.PHONY: test +.PHONY: test test-combine default: combine @@ -31,7 +35,8 @@ combine: mkbuild cp ${PATH_BROWSER_TEST} ${PATH_BUILD} # run tests -test: default $(TESTS) +test: default $(TESTS) test-combine +test-combine: default $(TEST_COMBINE) test-%.js: default node $@ test-%: default From 47a6ba27278423217bb73c061a904b05225b9041 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 12 Mar 2011 23:02:22 -0500 Subject: [PATCH 14/17] Added additional items to TODO for v0.1.0 --- TODO | 2 ++ 1 file changed, 2 insertions(+) diff --git a/TODO b/TODO index 1255025..0c9e606 100644 --- a/TODO +++ b/TODO @@ -6,6 +6,8 @@ Misc - Class module is becoming too large; refactor - Disallow member redeclaration in definition + - Permit binding on class methods + - Provide ability to free class from memory (class data stored in internal vars) Member Keywords - Restrictions; throw exceptions when unknown keywords are used From 6b374902aeb6f918f2824f4f1d76479247386534 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sat, 12 Mar 2011 23:48:38 -0500 Subject: [PATCH 15/17] Separated private members into a separate object (propobj) to prepare for future modifications - This incurs a performance hit for accessing protected members, and even further for public, internally - But speeds up access to private members, likely due to there being less members --- lib/class.js | 8 +++----- lib/propobj.js | 22 ++++++++++++++++++---- test/perf/perf-class-invoke-method.js | 4 ++++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/lib/class.js b/lib/class.js index cfae254..9933fdf 100644 --- a/lib/class.js +++ b/lib/class.js @@ -790,21 +790,19 @@ function attachPropInit( prototype, properties, members ) parent_init.call( this, true ); } + // this will return our property proxy, if supported by our environment, + // otherwise just a normal object with everything merged in var inst_props = propobj.createPropProxy( this, class_instance[ this.__iid ], properties[ 'public' ] ); - // use whatever was returned by the property proxy (which may not be a - // proxy, depending on JS engine support) - class_instance[ this.__iid ] = inst_props; - // if we're inheriting, perform a setup that doesn't include everything // that we don't want (e.g. private properties) var setup = ( inherit ) ? propobj.setupInherited : propobj.setup ; - setup( inst_props, properties, members ); + class_instance[ this.__iid ] = setup( inst_props, properties, members ); }); } diff --git a/lib/propobj.js b/lib/propobj.js index c60d431..5e25e91 100644 --- a/lib/propobj.js +++ b/lib/propobj.js @@ -39,7 +39,7 @@ var util = require( './util' ), * @param {Object} properties properties to copy * @param {Object=} methods methods to copy * - * @return {undefined} + * @return {Object} dest */ exports.setupInherited = function( dest, properties, methods ) { @@ -47,27 +47,41 @@ exports.setupInherited = function( dest, properties, methods ) // ensure we're not sharing references to prototype values doSetup( dest, properties[ 'public' ] ); doSetup( dest, properties[ 'protected' ], methods[ 'protected'] ); + + return dest; }; /** * Sets up properties (non-inheriting) * - * This includes all members (including private). + * This includes all members (including private). Private members will be set up + * in a separate object, so that they can be easily removed from the mix. That + * object will include the destination object in the prototype, so that the + * access should be transparent. This object is returned. * * @param {Object} dest destination object * @param {Object} properties properties to copy * @param {Object=} methods methods to copy * - * @return {undefined} + * @return {Object} object containing private members and dest as prototype */ exports.setup = function( dest, properties, methods ) { + // this constructor is an extra layer atop of the destination object, which + // will contain the private methods + var obj_ctor = function() {}; + obj_ctor.prototype = dest; + + var obj = new obj_ctor(); + // first, set up the public and protected members exports.setupInherited( dest, properties, methods ); // then add the private parts - doSetup( dest, properties[ 'private' ], methods[ 'private' ] ); + doSetup( obj, properties[ 'private' ], methods[ 'private' ] ); + + return obj; }; diff --git a/test/perf/perf-class-invoke-method.js b/test/perf/perf-class-invoke-method.js index bc8b5b8..2a97355 100644 --- a/test/perf/perf-class-invoke-method.js +++ b/test/perf/perf-class-invoke-method.js @@ -13,6 +13,10 @@ * - Protected and private methods internally should be accessed fairly * quickly since, like public methods externally, they are first on the * prototype chain. + * - Protected members will be accessed more slowly than private members, + * because they are one step lower on the prototype chain. Future versions + * will remove this performance hit if the Class contains no private + * members. * * Copyright (C) 2010 Mike Gerwitz * From e4e8900a9fc2c9c4971db13c5a6e9b20db6512c1 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 13 Mar 2011 03:55:43 -0400 Subject: [PATCH 16/17] Storing each supertype's private members in a separate object to prepare for future change - sid = super identifier --- lib/class.js | 21 +++++++++++---------- lib/propobj.js | 28 ++++------------------------ 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/lib/class.js b/lib/class.js index 9933fdf..16e5f44 100644 --- a/lib/class.js +++ b/lib/class.js @@ -774,10 +774,13 @@ function initInstance( iid, instance ) */ function attachPropInit( prototype, properties, members ) { - util.defineSecureProp( prototype, '__initProps', function( inherit ) + util.defineSecureProp( prototype, '__initProps', function( inherit, sid ) { - // defaults to false + // defaults to false, sid = super identifier inherit = !!inherit; + sid = ( sid ) ? +sid : 0; + + var iid = this.__iid; // first initialize the parent's properties, so that ours will overwrite // them @@ -787,22 +790,20 @@ function attachPropInit( prototype, properties, members ) // call the parent prop_init, letting it know that it's been // inherited so that it does not initialize private members or // perform other unnecessary tasks - parent_init.call( this, true ); + parent_init.call( this, true, ( sid + 1 ) ); } // this will return our property proxy, if supported by our environment, // otherwise just a normal object with everything merged in var inst_props = propobj.createPropProxy( - this, class_instance[ this.__iid ], properties[ 'public' ] + this, class_instance[ iid ], properties[ 'public' ] ); // if we're inheriting, perform a setup that doesn't include everything // that we don't want (e.g. private properties) - var setup = ( inherit ) - ? propobj.setupInherited - : propobj.setup - ; - class_instance[ this.__iid ] = setup( inst_props, properties, members ); + class_instance[ iid ][ sid ] = propobj.setup( + inst_props, properties, members, sid + ); }); } @@ -993,7 +994,7 @@ function getMethodInstance( method ) data = class_instance[ method.__iid ]; return ( iid && data ) - ? data + ? data[ 0 ] : null ; } diff --git a/lib/propobj.js b/lib/propobj.js index 5e25e91..7ea372d 100644 --- a/lib/propobj.js +++ b/lib/propobj.js @@ -30,28 +30,6 @@ var util = require( './util' ), ; -/** - * Sets up properties when inheriting - * - * This does not include private members. - * - * @param {Object} dest destination object - * @param {Object} properties properties to copy - * @param {Object=} methods methods to copy - * - * @return {Object} dest - */ -exports.setupInherited = function( dest, properties, methods ) -{ - // initialize each of the properties for this instance to - // ensure we're not sharing references to prototype values - doSetup( dest, properties[ 'public' ] ); - doSetup( dest, properties[ 'protected' ], methods[ 'protected'] ); - - return dest; -}; - - /** * Sets up properties (non-inheriting) * @@ -75,8 +53,10 @@ exports.setup = function( dest, properties, methods ) var obj = new obj_ctor(); - // first, set up the public and protected members - exports.setupInherited( dest, properties, methods ); + // initialize each of the properties for this instance to + // ensure we're not sharing references to prototype values + doSetup( dest, properties[ 'public' ] ); + doSetup( dest, properties[ 'protected' ], methods[ 'protected'] ); // then add the private parts doSetup( obj, properties[ 'private' ], methods[ 'private' ] ); From e03c081cfdc65e9c09761c09cc61e5bbc16b5206 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 13 Mar 2011 04:51:00 -0400 Subject: [PATCH 17/17] Fixed bug that made private members of a supertype inaccessible to methods that have not been overridden by a subtype - In doing so, abandoned the super identifier (sid) for a more elegant solution with class ids (cid's) - This permits fast and easy private member swapping via getMethodInstance() in class.js --- lib/class.js | 49 +++++++++++++-------- lib/member_builder.js | 14 +++--- test/test-class-visibility.js | 82 ++++++++++++++++++++++++++++++++++- 3 files changed, 119 insertions(+), 26 deletions(-) diff --git a/lib/class.js b/lib/class.js index 16e5f44..3aae42e 100644 --- a/lib/class.js +++ b/lib/class.js @@ -457,6 +457,9 @@ var extend = ( function( extending ) } } + // increment class identifier + class_id++; + util.propParse( props, { each: function( name, value, keywords ) { @@ -498,7 +501,8 @@ var extend = ( function( extending ) method: function( name, func, is_abstract, keywords ) { member_builder.buildMethod( - members, null, name, func, keywords, getMethodInstance + members, null, name, func, keywords, getMethodInstance, + class_id ); if ( is_abstract ) @@ -531,13 +535,13 @@ var extend = ( function( extending ) // set up the new class var new_class = createCtor( cname, abstract_methods, members ); - attachPropInit( prototype, prop_init, members ); + attachPropInit( prototype, prop_init, members, class_id ); new_class.prototype = prototype; new_class.constructor = new_class; // important: call after setting prototype - setupProps( new_class, abstract_methods, ++class_id ); + setupProps( new_class, abstract_methods, class_id ); // lock down the new class (if supported) to ensure that we can't add // members at runtime @@ -761,24 +765,24 @@ function initInstance( iid, instance ) * ensuring that their data is not shared with other instances (this is not a * problem with primitive data types). * - * The __initProps() method will also initialize any parent properties - * (recursive) to ensure that subtypes do not have a referencing issue, and - * subtype properties take precedence over those of the parent. + * The method will also initialize any parent properties (recursive) to ensure + * that subtypes do not have a referencing issue, and subtype properties take + * precedence over those of the parent. * * @param {Object} prototype prototype to attach method to * @param {Object} properties properties to initialize + * @param {number} cid class id * * @param {{public: Object, protected: Object, private: Object}} members * * @return {undefined} */ -function attachPropInit( prototype, properties, members ) +function attachPropInit( prototype, properties, members, cid ) { - util.defineSecureProp( prototype, '__initProps', function( inherit, sid ) + util.defineSecureProp( prototype, '__initProps', function( inherit ) { // defaults to false, sid = super identifier inherit = !!inherit; - sid = ( sid ) ? +sid : 0; var iid = this.__iid; @@ -790,7 +794,7 @@ function attachPropInit( prototype, properties, members ) // call the parent prop_init, letting it know that it's been // inherited so that it does not initialize private members or // perform other unnecessary tasks - parent_init.call( this, true, ( sid + 1 ) ); + parent_init.call( this, true ); } // this will return our property proxy, if supported by our environment, @@ -801,8 +805,8 @@ function attachPropInit( prototype, properties, members ) // if we're inheriting, perform a setup that doesn't include everything // that we don't want (e.g. private properties) - class_instance[ iid ][ sid ] = propobj.setup( - inst_props, properties, members, sid + class_instance[ iid ][ cid ] = propobj.setup( + inst_props, properties, members ); }); } @@ -980,21 +984,28 @@ function getMeta( id ) /** * Returns the instance object associated with the given method * - * The instance object contains the protected and private members. This object - * can be passed as the context when calling a method in order to give that - * method access to those members. + * The instance object contains the protected members. This object can be passed + * as the context when calling a method in order to give that method access to + * those members. + * + * One level above the instance object on the prototype chain is the object + * containing the private members. This is swappable, depending on the class id + * associated with the provided method call. This allows methods that were not + * overridden by the subtype to continue to use the private members of the + * supertype. * * @param {function()} method method to look up instance object for + * @param {number} cid class id * * @return {Object,null} instance object if found, otherwise null */ -function getMethodInstance( method ) +function getMethodInstance( inst, cid ) { - var iid = method.__iid, - data = class_instance[ method.__iid ]; + var iid = inst.__iid, + data = class_instance[ iid ]; return ( iid && data ) - ? data[ 0 ] + ? data[ cid ] : null ; } diff --git a/lib/member_builder.js b/lib/member_builder.js index 5daf630..3df3f4e 100644 --- a/lib/member_builder.js +++ b/lib/member_builder.js @@ -62,11 +62,12 @@ exports.initMembers = function( mpublic, mprotected, mprivate ) * @param {Object=} instCallback function to call in order to retrieve * object to bind 'this' keyword to + * @param {number} cid class id * * @return {undefined} */ exports.buildMethod = function( - members, meta, name, value, keywords, instCallback + members, meta, name, value, keywords, instCallback, cid ) { var prev; @@ -109,7 +110,7 @@ exports.buildMethod = function( if ( prev ) { // override the method - dest[ name ] = overrideMethod( prev, value, instCallback ); + dest[ name ] = overrideMethod( prev, value, instCallback, cid ); } else if ( keywords[ 'abstract' ] ) { @@ -120,7 +121,7 @@ exports.buildMethod = function( { // we are not overriding the method, so simply copy it over, wrapping it // to ensure privileged calls will work properly - dest[ name ] = overrideMethod( value, null, instCallback ); + dest[ name ] = overrideMethod( value, null, instCallback, cid ); } }; @@ -303,10 +304,11 @@ function scanMembers( members, name, cmp ) * * @param {Object=} instCallback function to call in order to retrieve * object to bind 'this' keyword to + * @param {number} cid class id * * @return {function()} override method */ -function overrideMethod( super_method, new_method, instCallback ) +function overrideMethod( super_method, new_method, instCallback, cid ) { instCallback = instCallback || function() {}; @@ -319,7 +321,7 @@ function overrideMethod( super_method, new_method, instCallback ) { override = function() { - var context = instCallback( this ) || this, + var context = instCallback( this, cid ) || this, retval = undefined ; @@ -344,7 +346,7 @@ function overrideMethod( super_method, new_method, instCallback ) // we are defining a new method override = function() { - var context = instCallback( this ) || this, + var context = instCallback( this, cid ) || this, retval = undefined ; diff --git a/test/test-class-visibility.js b/test/test-class-visibility.js index 61bf5c9..5c82bf2 100644 --- a/test/test-class-visibility.js +++ b/test/test-class-visibility.js @@ -73,6 +73,24 @@ var common = require( './common' ), { // override me }, + + + 'public getPrivProp': function() + { + return this.parts; + }, + + + 'public invokePriv': function() + { + return this._priv(); + }, + + + 'private _priv': function() + { + return priv; + }, }), // instance of Foo @@ -80,13 +98,28 @@ var common = require( './common' ), // subtype SubFoo = Foo.extend({ + 'private _pfoo': 'baz', + 'public getSelfOverride': function() { // return this from overridden method return this; }, + + + /** + * We have to override this so that 'this' is not bound to the supertype + */ + 'public getProp': function( name ) + { + // return property, allowing us to break encapsulation for + // protected/private properties (for testing purposes) + return this[ name ]; + }, }), - sub_foo = SubFoo() + sub_foo = SubFoo(), + + sub_sub_foo = SubFoo.extend( {} )() ; @@ -362,3 +395,50 @@ var common = require( './common' ), ); } )(); + +/** + * This one's a particularly nasty bug that snuck up on me. Private members + * should not be accessible to subtypes; that's a given. However, they need to + * be accessible to the parent methods. For example, let's say class Foo + * contains public method bar(), which invokes private method _baz(). This is + * perfectly legal. Then SubFoo extends Foo, but does not override method bar(). + * Invoking method bar() should still be able to invoke private method _baz(), + * because, from the perspective of the parent class, that operation is + * perfectly legal. + * + * The resolution of this bug required a slight system redesign. The short-term + * fix was to declare any needed private members are protected, so that they + * were accessible by the subtype. + */ +( function testParentMethodsCanAccessPrivateMembersOfParent() +{ + // properties + assert.equal( + sub_foo.getPrivProp(), + priv, + "Parent methods should have access to the private properties of the " + + "parent" + ); + + // methods + assert.equal( + sub_foo.invokePriv(), + priv, + "Parent methods should have access to the private methods of the parent" + ); + + // should apply to super-supertypes too + assert.equal( + sub_sub_foo.getPrivProp(), + priv, + "Parent methods should have access to the private properties of the " + + "parent (2)" + ); + assert.equal( + sub_sub_foo.invokePriv(), + priv, + "Parent methods should have access to the private methods of the " + + "parent (2)" + ); +} )(); +