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

Performance improvements by package updates / replacement #32

Merged
merged 10 commits into from
Apr 3, 2017

Conversation

fungilation
Copy link
Contributor

@fungilation fungilation commented Mar 19, 2017

  • node-lodash (deprecation warnings in npm) replaced with latest lodash
  • replaced sentence-tokenizer with sbd … about 50% faster performance, and better tokenizing! (don't consider stopwords like Mr. as end of sentences)
  • sentences_dict cached and passed to getSentencesRanks from public exports (.summarize, .getSortedSentences), to avoid reprocessing same content between calls
  • code styling based on JavaScript Standard Style
  • mocha, should updated
  • passing mocha tests: babel added for es6 imports, 3rd mocha test failed on undeclared var sorted_sentences... eliminated
  • also added yarn.lock, .babelrc

deprecated lodash aliases updated (pluck -> map, any -> some)
code style: semicolons not needed
between summarize, getSortedSentences calls of same content
sentences_dict cached and passed to getSentencesRanks from public exports
about 50% faster performance, and better tokenizing! (don't consider stopwords like Mr. as end of sentences)
@fungilation fungilation changed the title Package updates and replacement Performance improvements by package updates / replacement Mar 19, 2017
@jbrooksuk
Copy link
Owner

Can you also update the .travis.yml file so we can see tests pass, please? :)

- mocha "--compilers js:babel-core/register"
3rd mocha test failed on undeclared var sorted_sentences! Eliminated
@fungilation
Copy link
Contributor Author

Had to do more than just updating .travis.yml. All tests now passing.

A sidenote while I looked at optimizing performance for node-summary. I haven't dug much into the actual algorithm in summary.js, but note that I'm still having to resort to limiting summarization to first 70000 characters per article that I'm summarizing in my react native app. So as to limit processing time to roughly 5s per such 70000 chars text, as tested on a iPhone 6s.

Pointers to further optimization in the processing itself would be appreciated. Short of rewriting it in Swift/Java for mobile app use in my case...

added testing for LTS node (6.10.1 latest)
$ yarn
/home/travis/build.sh: line 229: yarn: command not found
The command "eval yarn " failed. Retrying, 2 of 3.
/home/travis/build.sh: line 229: yarn: command not found
The command "eval yarn " failed. Retrying, 3 of 3.
/home/travis/build.sh: line 229: yarn: command not found
The command "eval yarn " failed 3 times.
The command "yarn" failed and exited with 127 during .
Your build has been stopped.
@jbrooksuk jbrooksuk merged commit 5693056 into jbrooksuk:master Apr 3, 2017
@jbrooksuk
Copy link
Owner

Thanks for this!

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.

2 participants