Skip to content

Conversation

@RandomInsano
Copy link
Contributor

Help users learn how to directly write data to an object implementing std::io::Write.

Doing some tests with 1,000 counters:

  • Directly to a String: 454KB of RAM
  • Directly to a file: 305KB of RAM

Closes out #258

@RandomInsano
Copy link
Contributor Author

For the testing, I used heaptrack on Linux and read the "peak heap memory consumption" value. One one test of each was done.

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Small comment. Otherwise looks good to me. Thanks for the help!

Comment on lines 26 to 28
pub struct IoWriterWrapper<W>(W)
where
W: Write;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub struct IoWriterWrapper<W>(W)
where
W: Write;
pub struct IoWriterWrapper<W>(W);

Is the bound needed in the struct definition? It should only be needed in the std::fmt::Write implementation below.

Comment on lines 4 to 5
/// Example showing how one could write to a file or socket instead of a string.
/// For large metrics registries this will be more memory efficient
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would assume a module comment would be more appropriate. Feel free to ignore.

Suggested change
/// Example showing how one could write to a file or socket instead of a string.
/// For large metrics registries this will be more memory efficient
//! Example showing how one could write to a file or socket instead of a string.
//! For large metrics registries this will be more memory efficient.

@mxinden
Copy link
Member

mxinden commented Feb 25, 2025

Please address the CI failures.

@RandomInsano
Copy link
Contributor Author

RandomInsano commented Feb 25, 2025 via email

@mxinden
Copy link
Member

mxinden commented Feb 26, 2025

@RandomInsano just to avoid any confusion, you would need to make the above changes and then push another commit or amend your existing commit. Of course, there is no rush.

@RandomInsano
Copy link
Contributor Author

Thanks for the clarification! Code is pushed now. Verified the code still compiles and the assert still passes.

@RandomInsano
Copy link
Contributor Author

Missed the DCO failure. Decided to squash and force a new commit.

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done. Thanks.

@mxinden mxinden merged commit ab152ee into prometheus:master Feb 27, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants