Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a real world tiles bench #61

Merged
merged 1 commit into from
Aug 26, 2016
Merged

Add a real world tiles bench #61

merged 1 commit into from
Aug 26, 2016

Conversation

mourner
Copy link
Member

@mourner mourner commented Aug 26, 2016

I came to the conclusion that the benchmark in bench/bench.js doesn’t really make a lot of sense because V8 is extremely good at optimizing stuff that does the same thing over and over.

So I created a much more representative benchmark that downloads (and caches) 439 sample tiles from Mapbox Streets and runs decoding/encoding on them. Here’s a sample result:

439 tiles, 22612 KB total
read time: 405ms, or 54 MB/s
write time: 408ms, or 54 MB/s

I ran it on all commits between v2.0.1 and master and actually the most impactful change is the defaults PR. This is fine though — e.g. specifically for VT, you can optimize manually by definding a defaults object and then doing Object.create(defaults). This probably doesn’t make sense for the compiler to do though — the performance is more than great as it is.

cc @kjvalencik

@mourner mourner merged commit c871e71 into master Aug 26, 2016
@mourner mourner deleted the real-bench branch August 26, 2016 15:02
@kjvalencik
Copy link
Collaborator

This is great. I'll take a look and see if anything jumps out. The issue with switching to Object.create is that you won't be able to iterate them with things like Object.keys. The other issue is that you still need to create new arrays since they are modified in place.

I tried moving these to classes without any benefit before, but that was always creating new copies:

function Envelope {
    this.message = '';
}

This did not have any impact.

@mourner
Copy link
Member Author

mourner commented Aug 29, 2016

@kjvalencik yeah, I ended up dropping the Object.create idea because it speeds up things for this particular proto Value message but will get more problematic in other instances e.g. because you have to create an object first, and then set new properties to it (for arrays), which will switch hidden V8 classes. Also the added complexity doesn't seem to be worth the performance gain — the library is already fast enough for 99.9% cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants