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

Lazy load markup gems #231

Merged
merged 9 commits into from
Dec 4, 2013
Merged

Lazy load markup gems #231

merged 9 commits into from
Dec 4, 2013

Conversation

haileys
Copy link
Contributor

@haileys haileys commented Dec 3, 2013

github-markup currently eager loads markup gems when it's required. While this only takes about 100ms when testing the gem in isolation, when it's incorporated into a larger application like the GitHub rails app with a massive $LOAD_PATH, the time taken to load github-markup balloons out to 250-300ms.

This pull request adds lazy loading to github-markup so that markup gems are only required the first time they're used. For production environments where everything should be loaded up front, I've added a GitHub::Markup.preload! method that eager loads all markup gems straight away.

cc @github/perf

@haileys
Copy link
Contributor Author

haileys commented Dec 4, 2013

Ended up doing a bit of a refactoring heh. cc @github/markup for review

@haileys
Copy link
Contributor Author

haileys commented Dec 4, 2013

Also cc @defunkt and @gjtorikian since you two are the top two contributors

@gjtorikian
Copy link
Contributor

Wow, good stuff. 💖

Not even going to pretend I understand all the perf improvements, but I say :shipit:. I actually don't have publishing rights for this gem yet so I can't cut a new release, sadly. Seems it should warrant a minor version bump though.

@haileys
Copy link
Contributor Author

haileys commented Dec 4, 2013

@gjtorikian I think this deserves a major version bump (1.0.0 maybe?) because it does have breaking API changes.

Actually with that in mind, I might go and clean up a bit more cruft.

@haileys
Copy link
Contributor Author

haileys commented Dec 4, 2013

@gjtorikian Also the perf improvement is basically just not requiring gems until they're actually needed in development. In production, we'll still require them all up front.

haileys pushed a commit that referenced this pull request Dec 4, 2013
@haileys haileys merged commit 84523f5 into master Dec 4, 2013
@haileys haileys deleted the lazy-load-markups branch December 4, 2013 01:07
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