search

galkahana / HummusJS

Node.js module for high performance creation, modification and parsing of PDF files and streams
http://www.pdfhummus.com
Other
1.14k stars 169 forks source link

Changed the buffer size from 10Mb to 1K #250

Closed richard-kurtosys closed 6 years ago

richard-kurtosys commented 6 years ago

When using two streams as inputs, a 10Mb buffer is used to store the input stream. The output stream is then created from this which results in a 200k input being converted to a 10Mb output. While reducing the size of the buffer may cause the buffer to be reused more often, the output size is closer to the input size. Some benchmarks are available here: https://github.com/galkahana/HummusJS/issues/247

richard-kurtosys commented 6 years ago

The build error seems to be an issue unrelated to what I am trying to PR:

/Users/travis/.node-gyp/0.12.18/include/node/v8.h:5800:54: error: 'CreateHandle' is a protected member of 'v8::HandleScope'
return Handle<T>(reinterpret_cast<T*>(HandleScope::CreateHandle(

How can this be resolved?

galkahana commented 6 years ago

not gonna merge it. dont want to lower the buffer size. should fix the read stream you are using to read only the eligible count of bytes. it reads inAmount when it should limit the read data by the read data size.

galkahana commented 6 years ago

k. so:

  1. y'r apple issue is fine. a temp disagreement between osx and node. We're gonna have to live with that on 0.12. you can upgrade to a higher level of node to have it go away.
  2. the solution to this problem is not with the buffer size. rather, with the buffer stream. i included a version of it as part of hummus, which corrects teh problem.

Gal.