Inline i64tostr and itostr #13840

1. 
   Empact commented at 9:05 pm on August 1, 2018: member

   These methods don’t give us much, and removing them removes utilstrencodings’s dependence on tinyformat
2. MarcoFalke added the label Refactoring on Aug 1, 2018
3. Empact force-pushed on Aug 1, 2018
4. 
   DrahtBot commented at 10:05 pm on August 1, 2018: member

   • #13866 (utils: Use _wfopen and _wreopen on Windows by ken2812221)
   • #13862 (utils: drop boost::interprocess::file_lock by ken2812221)
   • #13846 (Move src/tinyformat.h to src/tinyformat/tinyformat.h by Empact)
   • #13845 (Include tinyformat as a subtree by Empact)
   • #13825 ([wallet] [Do not merge until v0.18] Kill accounts by jnewbery)
   • #13787 (Test for #13426 by ken2812221)
   • #13723 (PSBT key path cleanups by sipa)
   • #13671 (Remove the boost/algorithm/string/case_conv.hpp dependency by 251Labs)
   • #13426 ([bugfix] Fix encoding issue for Windows by ken2812221)
   • #13168 (Thread names in logs and deadlock debug tools (take 2) by jamesob)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

5. 
   practicalswift commented at 7:43 am on August 2, 2018: contributor

   utACK e00407dbbd13ccb8fd789811b4fee6871f737f9e
6. 
   laanwj commented at 10:23 am on August 2, 2018: member

   I’m not sure this is a good idea. I kind of liked having separate, specialized functions for this. These currently happen to be implemented in terms of strprintf, but this is not necessary and has relatively high overhead. (originally these were implemented using the appropriate C functions, but this was changed due to locale-dependence concerns)

   So tend toward NACK.

7. Empact force-pushed on Aug 2, 2018
8. Inline i64tostr and itostr

   These methods don't give us much, and removing them removes utilstrencodings's
   dependence on tinyformat

   3f4c6187a8
9. Empact force-pushed on Aug 2, 2018
10. 
    Empact commented at 3:24 pm on August 2, 2018: member

    I checked for cases in need of optimization and don’t really see them. Here are some numbers which show about an order of magnitude of variance: http://www.zverovich.net/2013/09/07/integer-to-string-conversion-in-cplusplus.html fmt seems to have the fastest implementation according to these: http://www.zverovich.net/2013/09/07/integer-to-string-conversion-in-cplusplus.html https://github.com/fmtlib/fmt#speed-tests but I doubt it’s worthwhile to bring that in.

    Looking into that led me to inlining WriteOrderPos and correcting the format string to be unsigned for ccode and nTransactionsUpdatedLast.

11. 
    donaloconnor commented at 9:54 pm on August 3, 2018: contributor

    I’m tending towards a Nack for this too unfort.

    Looking into that led me to inlining WriteOrderPos and correcting the format string to be unsigned for ccode and nTransactionsUpdatedLast.

    When yous say inlining, you mean in the code but the function is inlined anyway so run time it won’t have a performance impact. I like the functions since they document the code. It’s easier to read :-)

12. 
    Empact commented at 3:32 am on August 4, 2018: member

    Thanks for the look! Closing due to NACKs.
13. Empact closed this on Aug 4, 2018

    ---

14. MarcoFalke locked this on Sep 8, 2021

Contributors
Empact DrahtBot practicalswift laanwj donaloconnor

Labels
Refactoring