Skip to content

Performance fix for HTML Class. #1177

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

Closed
wants to merge 1 commit into from
Closed

Conversation

thybag
Copy link
Contributor

@thybag thybag commented Sep 4, 2012

Hello,

We've recently started using laravel for a number of internal web-apps, but have run in to a couple of issues when load testing. The underlying issue being that the config::get method is significantly more expensive than a standard array access. This is compounded by it being invoked a 100-1000 times per page load.

This pull request includes a change we've made to attempt to mitigate this issue, which functions by caching the application.encoding variable within the HTML class. To use one of our apps as an example, for a large form this reduced the number of calls to config::get from 1200+ invocations (72ms) to 125 (9ms).

All core unit-test's pass with this change in place.

Thank you,
Carl

…ls to Config::get();

Calling the "Config::get('application.encoding')" is expensive and within a large form (using the form builder) having it requested multiple times can result in a significant performance drag.

Caching this value reduced calls to Config:get within our project from 1200+ to 125. All core tests appear to pass with this change in place.
@travisbot
Copy link

This pull request passes (merged 6e44b40 into 1f005bd).

@thybag
Copy link
Contributor Author

thybag commented Sep 4, 2012

This pull request is targeting the wrong branch (master rather than develop) .
My new pull request is here: #1180

@thybag thybag closed this Sep 4, 2012
zoe-edwards pushed a commit to zoe-edwards/laravel that referenced this pull request Oct 14, 2013
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