Stash base dir paths in module constants #1416
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
|
Do you have an opinion @jneen? The idea makes sense to me but the overall reduction seems pretty small. |
pyrmont
added
maintainer-action
Memory profiling summary--- master https://travis-ci.org/rouge-ruby/rouge/jobs/646657531
+++ PR https://travis-ci.org/rouge-ruby/rouge/jobs/646846491
- Total allocated: 13.90 MB (140145 objects)
+ Total allocated: 13.86 MB (139773 objects)
- Total retained: 5.32 MB (37776 objects)
+ Total retained: 5.32 MB (37780 objects) |
pyrmont
force-pushed
the
dirname-constants
branch
from
a6857a6
to
68565e4
Compare
pyrmont
removed
the
maintainer-action
pyrmont
approved these changes
Apr 3, 2020
|
@ashmaroli I realise you split #1430 off from this one so that #1430 could more easily be merged. I'd still like to discuss that one but having looked at this, I think it's helpful in more clearly expressing intent. And, as you pointed out, it reduces memory usage! Thanks for submitting it :) |
mattt
pushed a commit
to NSHipster/rouge
that referenced
this pull request
May 21, 2020
…-ruby#1416) Rouge makes use of dynamic loading of files at various points throughout its codebase. In these instances, the method `Kernel#__dir__` is used. This commit instead uses module constants to more clearly express intent and, as a bonus, reduce the number of object allocations required.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
__dir__allocates a new string on each call. Using this method in string interpolation(
"#{__dir__}/....") or path based methods (File.join(__dir__, ....), etc) results in duplication of the newly generated string.This allocation can be reduced by stashing the value in frozen module / class constants.