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

[don't merge] Loading faces from memory: super slow? #81

Merged
merged 3 commits into from
Mar 23, 2015
Merged

Conversation

lbud
Copy link
Contributor

@lbud lbud commented Mar 20, 2015

No description provided.

@springmeyer
Copy link
Contributor

My results locally do not indicate a slowdown for range. I think the load is running so fast there is a lot of noise so take the differences with a grain of salt.

overhaul

fontnik.load 1186 ops/sec 10 10
fontnik.range 22 ops/sec 1000 100

overhaul-memory

fontnik.load 2825 ops/sec 10 10
fontnik.range 21 ops/sec 1000 100

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.77%) to 94.89% when pulling 87fbd84 on overhaul-memory into d67e56b on overhaul.

@springmeyer
Copy link
Contributor

bench2.js results:

overhaul

fontnik.load x 509 ops/sec ±1.62% (45 runs sampled)
fontnik.range x 9.54 ops/sec ±1.82% (49 runs sampled)

overhaul-memory

fontnik.load x 531 ops/sec ±1.46% (44 runs sampled)
fontnik.range x 9.44 ops/sec ±1.87% (49 runs sampled)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.77%) to 94.89% when pulling 60593bd on overhaul-memory into d67e56b on overhaul.

 - avoid allocating std::string and instead keep alive the node.Buffer
   containing the in-memory font data and use a pointer to its memory
 - use RAII principles in baton structs for setting up and destroying persistent handles
@springmeyer
Copy link
Contributor

@lbud - just pushed a few minor improvements to this branch. To recap: I'm not seeing any slowdowns, so I think this is ready to merge if you can confirm the same. Interested to hear if you can still replicate slowdowns.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.56%) to 95.1% when pulling 7d39a04 on overhaul-memory into d67e56b on overhaul.

@lbud
Copy link
Contributor Author

lbud commented Mar 23, 2015

Aha, weird — ran both this morning and didn't see any slowdowns either once I reran npm install --build-from-source. I'm wondering if something about the way I had compiled it in debug mode before slowed it down, or if it was just from the million unresponsive atom helper process I had in the background on Friday (:unamused:). So, good to merge.

lbud pushed a commit that referenced this pull request Mar 23, 2015
Load faces from memory rather than disk
@lbud lbud merged commit d6035c4 into overhaul Mar 23, 2015
@lbud lbud deleted the overhaul-memory branch March 23, 2015 20:01
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.

3 participants