strongloop-community / loopback-benchmarks

https://github.com/strongloop-internal/scrum-cs/issues/54
0 stars 0 forks source link

Candidates for performance optimization #1

Open raymondfeng opened 9 years ago

raymondfeng commented 9 years ago
bajtos commented 9 years ago

https://github.com/strongloop/loopback/blob/master/server/middleware/rest.js#L27 we should build the handlers once at middleware level instead of per request)

That's already happening, see L38.

https://github.com/strongloop/strong-remoting/blob/master/lib/http-context.js#L68 Remove the usage of closure, use for-loop instead

IMO it's questionable how much performance we can gain by that change. We should measure the performance of both versions and pick up the fastest one.

Remove the need to bind(this)

Definitely, we should use self instead. We should start by fixing this, and only later look into replacing forEach with for-loop.

bajtos commented 9 years ago

https://github.com/strongloop/loopback/blob/master/server/middleware/rest.js#L27 we should build the handlers once at middleware level instead of per request)

That's already happening, see L38.

Having said that, we are creating a new array for each request later on L61 (preHandlers.concat(restHandler)). This should be definitely optimised.

raymondfeng commented 9 years ago

for-loop is much faster than Array.prototype.forEach, see http://jsperf.com/fast-array-foreach. I already used the same technique to optimize juggler before.

raymondfeng commented 9 years ago

L38 doesn't help as the declaration of preHandlers at line 36 is scoped to the handler function, not the middleware factory. As a result, L38 is always evaluated to true.

raymondfeng commented 9 years ago

We should also look into usages that prevent V8 optimization - https://github.com/petkaantonov/bluebird/wiki/Optimization-killers.

https://www.youtube.com/watch?v=NthmeLEhDDM

bajtos commented 9 years ago

L38 doesn't help as the declaration of preHandlers at line 36 is scoped to the handler function, not the middleware factory. As a result, L38 is always evaluated to true.

Good catch!

raymondfeng commented 9 years ago

Another PR - https://github.com/troygoode/node-cors/pull/35