Replace github.com/hashicorp/golang-lru with a custom lru cache #4196
Conversation
aarzilli
left a comment
aarzilli
left a comment
There was a problem hiding this comment.
Overall this looks correct to me.
|
If we decide to switch from hashicorp/lru to this (I have no opinion in merit, I leave it to Derek) I think we should have some tests. |
Added some basic tests |
derekparker
left a comment
derekparker
left a comment
There was a problem hiding this comment.
Overall looks great, thanks!
You can fix the capslock CI failures by running go run _scripts/gen-capslock-all.go locally (need to update the test output... and very frustrating that they seemed to have changed their output format for capability names). Also, I feel that this new code still deserves its own package, so can you move your LRU implementation & tests into pkg/proc/internal/lru (or similar).
|
@qmuntal hey just wanted to ping if you have time to address the above. Thanks! |
Wops, forgot about this PR. Addressing your comments right now. |
3 participants
(Feel free to disregard this PR is you don't like what's in in)
Delve uses a LRU cache to speed up some workflows. It just makes a basic use of it: adding and getting entries. Implementing that subset using the Go standard library is straightforward, it can be done in 70 lines of code. IMO dropping an external dependency is worth the small maintenance overhead of implementing a custom basic LRU cache.