Optimize Doc::Method#compute_doc_info to avoid duplicate regex
#13324
Conversation
| return DocInfo.new(def_doc, nil) | ||
| end | ||
| # TODO: warn about `:inherit:` not finding an ancestor | ||
| inherit_def_doc = def_doc.gsub(/^[ \t]*:inherit:[ \t]*$/m) { ancestor_doc_info.try(&.doc) || break } |
There was a problem hiding this comment.
This would recompute #ancestor_doc_info on every iteration. Normally docs shouldn't contain more than one :inherit: but it doesn't hurt to cache that in a local variable
There was a problem hiding this comment.
Now it's still being recomputed if self.inherit_def_doc returns nil. This double meaning of nil is also bad for understanding the code. I'd just drop the caching (revert that commit).
There was a problem hiding this comment.
Now it's still being recomputed if
self.inherit_def_docreturnsnil. This double meaning ofnilis also bad for understanding the code. I'd just drop the caching (revert that commit).
(this issue still stands; reformatting didn't change the behavior)
There was a problem hiding this comment.
Ah sorry, I missed your comment here @oprypin
I suppose you mean self.ancestor_doc_info?
That doesn't get recomputed because when it returns nil, the block breaks and there's no repeat iteration.
The algorithm executed basically the same regex twice: once to see whether it matches and then to replace all occurrences.
This patch reduces it to a single regex match by avoiding the initial check.