takuyaa / kuromoji.js

JavaScript implementation of Japanese morphological analyzer
848 stars 118 forks source link

Use built-in zlib module instead of zlib.js #6

Closed xl1 closed 9 years ago

xl1 commented 9 years ago

https://nodejs.org/api/zlib.html

This will improve performance to decompress dictionaries on Node.js environment.

I measured loader performance by a script below:

console.time("load");
var loader = DictionaryLoader.getLoader(DIC_DIR);
loader.load(function (err, dic) {
    console.timeEnd("load");
});

And the results (Windows 10, Node v0.12.6):

load: 1951ms
load: 1875ms
load: 1891ms
load: 1892ms
load: 1947ms
load: 1234ms
load: 1218ms
load: 1223ms
load: 1266ms
load: 1268ms
takuyaa commented 9 years ago

Thank you for your contribution to kuromoji.js! It seems to be very nice PR for me, but have some problems.

Names of normal variable (not class) are not camelCase but snake_case as you see in kuromoji.js repo. For consistency, could you change variable names? (ex. nodeZlib to node_zlib and typedArray to typed_array )

In addition, you don't need to update files under dist/ and jsdoc/ directory. I think these files should be updated when we tag a version (and release it).

If you could modify (or revert) the 2 above, I am ready to merge this PR.

xl1 commented 9 years ago

Thank you for replying! I've fixed them.

takuyaa commented 9 years ago

I appreciate your quick fix!