(#6015) - use Map and Set in mapreduce #6015

Merged
merged 1 commit into from Dec 21, 2016

Projects

None yet

2 participants

@nolanlawson
Member

Been trying to find ways to improve perf for secondary indexes, this seemed like an obvious change given #5991 showed that the native Map/Set implementations are faster than our shims. Citation needed, but should be a perf boost.

Rebased on #5991 to avoid bitrotting myself.

@nolanlawson
Member

BTW one major reason I'm doing this is that during my perf analyses I see lots of time spent in garbage collection, so I'm hunting down unnecessary allocations. This uniq() function was a major offender, but after this fix it barely registers.

@nolanlawson
Member

True failure; PhantomJS is not fond with how we're using Map and Set.

@nolanlawson
Member

Hm, I can repro the Phantom failure in Safari 5, but not in Safari 6. And I'm having a heck of a time running PhantomJS locally.

@nolanlawson
Member

Awesome, this actually revealed a bug in our Map/Set implementation that wouldn't have been revealed otherwise. Also it turns out Safari 5 running on Windows 7 in VirtualBox is a good way to debug PhantomJS errors!

This should be green after the next run.

@nolanlawson nolanlawson (#6015) - use Map and Set in mapreduce
51aaa2d
@daleharvey daleharvey merged commit 674e908 into master Dec 21, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment