First round of comments:
https://chromiumcodereview.appspot.com/9361038/diff/4002/benchmarks/navier-stokes.js File benchmarks/navier-stokes.js (right): https://chromiumcodereview.appspot.com/9361038/diff/4002/benchmarks/navier-stokes.js#newcode27 benchmarks/navier-stokes.js:27: var name = 'navier-stokes2'; I think you should make this fit with the other benchmarks in the V8 benchmark suite before submitting it. I don't think you need the name variable. You'll need to create a setup, run, and tear down function (you almost have these). The tear down function should reset the global |solver| variable to undefined or null so that a GC will get rid of the FluidField object. Also it would be good to make sure to update the documentation (in the various html and readme files). https://chromiumcodereview.appspot.com/9361038/diff/4002/benchmarks/navier-stokes.js#newcode31 benchmarks/navier-stokes.js:31: solver = new FluidField(null); The |solver| variable is undeclared. Add a global solver variable with 'var'. Another thing: Could we make this 2 space indent instead of 4? https://chromiumcodereview.appspot.com/9361038/diff/4002/benchmarks/navier-stokes.js#newcode39 benchmarks/navier-stokes.js:39: function runNavierStokes() I would replace this entire method with a call to solver.update() and use the benchmark harness from base.js to drive it. https://chromiumcodereview.appspot.com/9361038/diff/4002/benchmarks/navier-stokes.js#newcode54 benchmarks/navier-stokes.js:54: // This code is copied from Oliver's html page. Reference to the exact page? https://chromiumcodereview.appspot.com/9361038/diff/4002/benchmarks/navier-stokes.js#newcode68 benchmarks/navier-stokes.js:68: var x1 = [329, 312, 62, 326, 96, 480, 359, 487, If any of these variables change during the execution of the benchmark, it would be nice to make sure they are initialized in the setup function. https://chromiumcodereview.appspot.com/9361038/diff/4002/benchmarks/navier-stokes.js#newcode77 benchmarks/navier-stokes.js:77: var pointCount = 0; Is there anyway you can check that the benchmark behaves correctly based on some of these counters? In general, it is much better if the benchmarks do at least a certain amount of checking (some of it might be postponable to the tear down face). https://chromiumcodereview.appspot.com/9361038/diff/4002/benchmarks/navier-stokes.js#newcode93 benchmarks/navier-stokes.js:93: setupNavierStokes(); These two calls should be dealt with by the benchmark harness. https://chromiumcodereview.appspot.com/9361038/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
