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

Replace DataView.getUint8 by typed array access #7

Closed
wants to merge 2 commits into from

Conversation

letmaik
Copy link
Contributor

@letmaik letmaik commented Dec 11, 2015

Using DataView is currently still slower than directly accessing a typed array. For the case of the expression dataView.getUint8(offset) it is possible to circumvent the use of DataView and directly use a typed array. This is possible since the DataView is initialized with a 1 byte element length, which makes it equivalent to a direct Uint8Array view of the data.

With this change, there are speed increases of 15% on Chrome 47 and 36% on Firefox 42 on my machine while decoding around 200MiB of data. Testing was done with a very large array of numbers where one third of elements were nulls.

I am working with such big data, so I would be happy if this is merged. Typically the data I have compresses extremely well (60MiB get down to 2MiB after gzipping in my case) so the bottleneck is the CBOR decoding after the data is unzipped. And this change is one step in making cbor-js a bit faster.

Test code:

var data = new Array(1024*1024*25) //*8 ~ 200MiB
for (var i=0; i<data.length; i++) {
  data[i] = i % 3 === 0 ? null : i + 0.1
}
var obj = {
  data: data
}  
var cborData = CBOR.encode(obj)

var runs = 3
var t0 = new Date()
for (var i=0; i<runs; i++) {
  CBOR.decode(cborData)
}
console.log((new Date()-t0)/runs + 'ms')

With this change, there are speed increases of 15% on Chrome 47 and 36% on Firefox 42 while decoding around 200MiB of data. Testing was done with a very large array of numbers where one third of elements were nulls.
@paroga paroga force-pushed the master branch 2 times, most recently from 06af6e4 to ca05798 Compare September 14, 2016 15:49
@paroga
Copy link
Owner

paroga commented Sep 14, 2016

Sorry for the very very long silence. Does this change still make sense? Would a similar change for 16bit/32bit values increase the performance too?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 99.265% when pulling 46cd936 on letmaik:uint8-optimize into 66ccfe3 on paroga:master.

@letmaik
Copy link
Contributor Author

letmaik commented Sep 14, 2016

I just merged in the latest changes from master and repeated the benchmarks. The speed increases are still in the same ranges on both Firefox (48) and Chrome (52). Therefore I would still think that it makes sense. You cannot do the same optimization on the other data types as you wouldn't easily know the correct offset into the typed array view.

@TYoung86
Copy link
Contributor

@paroga This change couldn't apply precisely to 16-bit and 32-bit values as the alignments would vary, 16-bit and 32-bit values are packed unaligned with their 8-bit prefixes, but an alignment check could be added before attempting a read; though the alignment check could offset the benefit of just using the DataView. The abundance of single-byte reads is easy enough to warrant this change.

@TYoung86
Copy link
Contributor

@letmaik encoding, writeUint8, and writeUint8Array could likely benefit from this tweak

@letmaik
Copy link
Contributor Author

letmaik commented Jan 26, 2019

This is getting old. Please merge it or I'll close the PR as I'd like to delete my fork of cbor-js.

@letmaik letmaik closed this Jul 16, 2022
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

4 participants