Fix memory usage#3
Conversation
AlessioRocco
left a comment
AlessioRocco
left a comment
There was a problem hiding this comment.
Solid work. The core memory fix is correct and well-executed — switching from character-by-character iteration to boundary-based slicing is exactly the right approach, and the use of String.new + << throughout is consistent and intentional. The hashery removal is clean and the plain-Ruby LRU reimplementation is sound. One nit inline, otherwise happy to approve.
| @intern.delete(key) | ||
| @intern[key] = { data: value, expired_in: expired_in && Time.now.getutc.to_i + expired_in } | ||
| @intern.delete(@intern.keys.first) while @intern.size > @max_size | ||
| value |
There was a problem hiding this comment.
Nit: @intern.keys.first allocates a full keys array on every call. Prefer @intern.shift — it pops the oldest entry (insertion-order head) in-place without the intermediate allocation:
@intern.shift while @intern.size > @max_size
schimpf
left a comment
schimpf
left a comment
There was a problem hiding this comment.
Code checked, looks good. Also tests are passing. We have to do a big regression test on the marketplace repot when we use this though.
3 participants
This PR changes Fragments::StructuredText#as_html to reduce memory allocation. The original implementation used an extremely inefficient character-by-character string concatenation.