Skip to content

Instantly share code, notes, and snippets.

@DavidBruant
Created August 20, 2013 15:37
Show Gist options
  • Save DavidBruant/6283118 to your computer and use it in GitHub Desktop.
Save DavidBruant/6283118 to your computer and use it in GitHub Desktop.

// Nothing here -- never executed. => Why a function then? Is it ever used even as a constructor? If not, a singleton might be what you need.

isPrototypeOf => Why not instanceof?

"Could as well be defined as an interface" => There is this TypeScript thing. You should check it out ;-)

Math methods => Maybe the deferencing takes time and you should put them on top. Not sure.

typeof is a bitch => You can replace that test with: Object(x) === x

risk of infinite loop => I imagine you don't use it this way, but the traverse function can do an infinite loop if you have circular references. Also, you may want to protect the .call with a try/catch block so that one error thrown doesn't stop the traversing. But try/cacth hurts perf I heard, so I'll let you judge whether it's appropriate.

Une fonction de trop => return window.performance.now.bind(performance)

rAF polyfill => Interesting. I think the value passed to the callback is buggy. You may want to call getTimestamp inside the setTimeout callback so you can pass a value as accurate as possible given on when the function is actually called (instead of scheduled to be called) You may also want to update lastTime inside the setTimeout callback. Let's put some code on what I would expect:

var lastTime = 0;
this.requestAnimationFrame = function(callback) {
    var currTime = JSSMS.Utils.getTimestamp();
    var timeToCall = Math.max(0, 1000/60 - (currTime - lastTime));
    window.setTimeout(function() {
        lastTime = JSSMS.Utils.getTimestamp();
        callback(lastTime);
    }, timeToCall);
};

Maybe rAF should be part of Utils? (hiding prefix and polyfill bullshit in there)

jQuery => Given you're certainly targeting only modern browsers, you may not really need it ;-)

Love the AST-based JS generation code.

@thom4parisot
Copy link

Actually recently I've learned the ~~ trick to floor numbers faster. It can be a good way to improve the performace without degrading much the readability.

~~0.5 === 0
Math.floor(0.5) === 0

@gmarty
Copy link

gmarty commented Aug 28, 2013

Hey! Many thanks!

// Nothing here -- never executed.
=> Why a function then? Is it ever used even as a constructor? If not, a singleton might be what you need.

Very good point. It's actually just used to augment Z80 class (see here). But I retain the object syntax to keep in sync with the other classes... what makes no sense. I'd fix it, but this debugger will probably be deleted in the future.

isPrototypeOf
=> Why not instanceof?

Blindly copy/paste-ed from n64js. I fixed it.

"Could as well be defined as an interface"
=> There is this TypeScript thing. You should check it out ;-)

I'm working on another emulator written in TS and I love it.

Math methods
=> Maybe the deferencing takes time and you should put them on top. Not sure.

Possibly, but in real conditions, this method is very rarely called, so it doesn't even worth testing.

typeof is a bitch
=> You can replace that test with: Object(x) === x

OK, thanks for the tip. Ultimately, that bit will be replaced with estraverse.

risk of infinite loop
=> I imagine you don't use it this way, but the traverse function can do an infinite loop if you have circular references.
Also, you may want to protect the .call with a try/catch block so that one error thrown doesn't stop the traversing. But try/cacth hurts perf I heard, so I'll let you judge whether it's appropriate.

Same as above. This piece is absolutely crucial for performance, so I prefer no to use try/catch. There shouldn't be any circular references anyway. I really need to use estraverse

Une fonction de trop
=> return window.performance.now.bind(performance)

Exactly what I needed. Thanks for the tip, I love this neat syntax.

rAF polyfill
=> Interesting. I think the value passed to the callback is buggy. You may want to call getTimestamp inside the setTimeout callback so you can pass a value as accurate as possible given on when the function is actually called (instead of scheduled to be called)
You may also want to update lastTime inside the setTimeout callback.

I used your code and it works like a charm. I noticed it runs lightly faster than 60fps, but that's fine.
There's no need to pass an argument to the callback.

Maybe rAF should be part of Utils? (hiding prefix and polyfill bullshit in there)

Good idea, indeed.

jQuery
=> Given you're certainly targeting only modern browsers, you may not really need it ;-)

I'm using jQuery to a lot of different places, so it's not trivial to simply ditch it.

Love the AST-based JS generation code.

I love it too now that it is working. But it's very hard to debug, specially because escodegen errors are not user friendly (there's maybe a way to turn on meaningful error messages).

Thanks again for your help, I really appreciate.

@gmarty
Copy link

gmarty commented Aug 28, 2013

And thanks @oncletom, I've used ~~ for Math.floor(). If it's fast and easy to read, why not using it? :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment