-
Notifications
You must be signed in to change notification settings - Fork 246
Move memory stats gathering from polkadot to parity-util-mem
#588
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
Conversation
a3da089 to
2b1d61e
Compare
2b1d61e to
3c2798f
Compare
drahnr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
ordian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Could you also update the changelog in parity-util-mem?
Sure thing! |
dvdplm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few undocumented items and I think I agree with @ordian about preferring an Err to a panic.
Since one cannot create the item itself ( |
If by undocumented you mean the lack of doc comments then yes, these are deliberately undocumented since the documentation's supposed to be on the facade (since that's what's supposed to define the public API; not much point in copy-pasting exactly the same thing for crate internal structures).
Well, it's not supposed to be ever called, which is why I think an |
|
FWIW, I'm fine with the current impl, was reviewing it in the pre-coffee state at first 😅 |
|
Okay, I've updated the changelog (and I've also backfilled the release date of the If you'd like anything else changed now's the time to speak up. (: |
Sorry, do you want me to publish it? Maybe I misinterpreted your message. But feel free to do it, you should have the rights to publish to crates.io. |
|
Sorry, no, I'll do it myself next week; I was just busy with the RPC memory leak, which is why I didn't do it immediately. (: |
I want to switch the jemalloc crate from
jemallocatortotikv-jemallocator, howeverpolkadotdirectly depends on thejemalloc-ctlcrate which makes such a switch a breaking change (sincejemalloc-ctlpulls the originaljemalloc-sys, and you can't have two jemallocs compiled in at the same time).So this moves that jemalloc-specific chunk of code (originally introduced in paritytech/polkadot#3612) from
polkadothere, and abstracts it away.