perf optimization about geofire-js HOT 5 CLOSED

@bolshchikov - Thanks a lot for the suggestions! I'll take a look at each one this week and do some perf testing to see how much they help.

from geofire-js.

@bolshchikov - Thanks again for the perf suggestions. I implemented numbers 1 and 2 and they are currently awaiting code review in pull request #52. I did not implement suggestion 3 since those two operations are not the same. If you just set it to undefined, then hasOwnProperty() will return true. But when we delete, hasOwnProperty() returns false. I don't think adding a bunch of checks to fix this discrepancy is worth the potential speed improvement. Although I do think there would be some speed improvement, so thanks for alerting me of that.

At this point, I think most of the perf gains we will get in GeoFire will come from improvements to the underlying Firebase library itself. We will continue to make that faster so that the libraries that build on top of it are even faster!

I'll keep this task open until we ship a version with these changes in them. Which should happen in a day or so.

from geofire-js.

@jwngr, great improvements, but the regarding the second, I've suggested to use lodash library for perf method optimization since it's the target of the library. Iterating over Object.keys is more effective however native .forEach is way slower than a for loop iteration.
So ideally the code would look like the following

var i = 0, arr = Object.keys(foo), len = arr.length;
for (i; i < len; i ++) { /* access the property: foo[arr[i]] */ }

So here, we defined variables outside of the loop to prevent from recalculation and using the regular for loop. This performance will be the best.

Regarding the delete vs undefined, there are many talk about it (see this thread). In short, V8 optimizes JS code, and call of delete causes the change of the object which leads to deoptimization, which in turn leads to degradation up to 12x.

I can go over the code and make a PR if you like.

from geofire-js.

I've updated the pull request again. We are not going to include the lodash library itself because the perf benefits it brings are outweighed by the fact that we now have an extra dependency and increased file size. I definitely like the idea of using the tricks it uses to speed up the code though. Thanks for sharing.

from geofire-js.

These improvements were released in version 3.0.3. Thanks again!

from geofire-js.