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

Moving text width measure to it's own function, with cache #469

Closed
wants to merge 1 commit into from

Conversation

ElNounch
Copy link
Contributor

@ElNounch ElNounch commented Jun 9, 2015

No description provided.

@espadrine
Copy link
Member

Thanks for contributing!

Do you have benchmark figures that justify the cost of RAM?

As an aside, all of the RAM of our current Heroku instances is currently allocated, so we'll have to wait for us to move to new servers to merge this.

@AdrieanKhisbe
Copy link
Contributor

now we are not on heroku I think we can merge it :)

@espadrine
Copy link
Member

I'd like to further compare this to what #496 gives, performance-wise, both to determine if it would have any impact on performance (whether positive or negative), and to see if a switch to pdfkit doesn't simply kill that bird too with one stone. Going from a median of 70ms to one of 20ms may give way better results than this patch, which will only speed up 256 strings.

(All performance-related patches take quite a bit of time to process, as they often require benchmarks and tweaks.)

@paulmelnikow paulmelnikow added enhancement performance-improvement Related to performance or throughput of the badge servers labels Mar 27, 2017
@olivierlacan
Copy link
Member

There's no feedback from @ElNounch in nearly two years. I'm going to close this and @espadrine can re-open it if #496 didn't provide the benefits this offers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance-improvement Related to performance or throughput of the badge servers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants