Not sure if these should be separate PRs.
First is removal of a platform abstraction for getting cycle counters where possible. Since the benchmarking switch to counting number of iterations over a fixed window instead of counting cycles per iteration, these are unused.
Second is removal of a few methods from the Node interface that seem vestigial from when the concepts of wallet/node were not as clearly separated.
Concept ACK
Last use of perf_cpucycles was removed in 00721e69f8280f8bc59bede43b335ecc347d4fdf. getFallbackFee, getPayTxFee and setPayTxFee seem to never have been used.
@tjps, very nice finds! How did you find these? Did you use any tool?
utACK 1bf3f33b4661addf93669747b32a07065f17a551
that seem vestigial from when the concepts of wallet/node were not as clearly separated.
I take this as a bit of an insult because I added these methods for #10102 process separation, and definitely had a clear concept of what needed to go in the node and wallet processes :smiley:.
But it is true that these methods though are no longer needed after 1983ca6cb3d6e741191206b57585a4b88d9ab86e in #10706, when the sendcoins dialog switched from getting and setting the global transaction fee to using coincontrol, and after 2fffaa97381f741786fff2e6ff25f4b9a74037fe in #10706 when it stopped displaying the fallback fee.
Good finds!
utACK 1bf3f33b4661addf93669747b32a07065f17a551
NACK on removing perf.cpp/.h
Since the benchmarking switch to counting number of iterations over a fixed window instead of counting cycles per iteration, these are unused.
WTF - how is that a reason to remove cycle-counting? Cycles/iteration would still be a useful metric.
For future reference there was a discussion on irc:
https://botbot.me/freenode/bitcoin-core-dev/2018-04-19/?msg=99157783&page=2
Using cycles could be done, but it likely wouldn't add useful information, since the std::chrono clock might be using cycles internally (or something that is equivalent)
Hi all, I want to clarify that I meant no disrespect in any of my comments. @practicalswift - I don't have any one specific method for finding dead code. It's just a passion project for me -- the only thing better than perfect code is no code at all. I have a collection of homegrown tools I have built over the years, and I use them in a tech debt consultancy I operate (sadly still no public showcase yet). @ryanofsky - I didn't mean to imply that they were vestigial at the time of move-over. I saw your commit that moved them, then verified that the functionality existed in the wallet in current HEAD and axed it at that point. @laanwj - for the record I really liked the architecture-abstracted cycle counter utility, I see you are the one who added it. My view is that the current tree should be kept to the minimum possible working set, and good code can always be brought back from the git history if needed. To me code removal is not a statement on its usefulness, but instead a statement on if it is used.
@laanwj - for the record I really liked the architecture-abstracted cycle counter utility, I see you are the one who added it. My view is that the current tree should be kept to the minimum possible working set, and good code can always be brought back from the git history if needed. To me code removal is not a statement on its usefulness, but instead a statement on if it is used.
Thanks, yes I agree, it's not your fault anyhow! - I apparently didn't pay enough on the review of the PR which removed it in the first place, to which I did utACK. We should have had a proper discussion there. Though the end-result could have been just the same...
Using cycles could be done, but it likely wouldn't add useful information, since the std::chrono clock might be using cycles internally (or something that is equivalent)
I more or less agree, though I also have to add that when using low-level measurement primitives it is transparent what is being measured all the way to the kernel/hardware, with the whole c++ abstraction stack it's a lot less clear what is actually used (being c++ library dependent). I guess all in all in practice this is good enough, in the common case, but when e.g. optimizing the assembly implementation of an algorithm it's clear which one to pick.
I don't have any one specific method for finding dead code. It's just a passion project for me -- the only thing better than perfect code is no code at all. I have a collection of homegrown tools I have built over the years, and I use them in a tech debt consultancy I operate (sadly still no public showcase yet).
Very nice! I share that passion! :-)
Please keep searching for dead code!