Print size of pcoinsTip cache in AcceptToMemoryPool #7150

pull pstratem wants to merge 2 commits into bitcoin:master from pstratem:2015-12-1-printcachesize changing 1 files +3 −2

pstratem commented at 9:57 PM on December 1, 2015: contributor

What the title says.
laanwj added the label Mempool on Dec 2, 2015
Print size of pcoinsTip cache in AcceptToMemoryPool dc4a342416
Normalize AcceptToMemoryPool log message to MiB from kB 9773066ad5
in src/main.cpp:None in 9773066ad5
```
4726 | @@ -4727,10 +4727,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
4727 |              RelayTransaction(tx);
4728 |              vWorkQueue.push_back(inv.hash);
4729 |  
4730 | -            LogPrint("mempool", "AcceptToMemoryPool: peer=%d: accepted %s (poolsz %u txn, %u kB)\n",
4731 | +            LogPrint("mempool", "AcceptToMemoryPool: peer=%d: accepted %s (poolsz %u txn, %u MiB, cache=%.1fMiB(%utx))\n",
```
morcos commented at 3:04 AM on December 8, 2015:

sorry to be annoying, but might be nice if they matched up.. maybe: LogPrint("mempool", "AcceptToMemoryPool: peer=%d: accepted %s (poolsz %u txn, %.1f MiB, cache %u txn, %.1f MiB)\n"

MarcoFalke commented at 9:53 AM on December 8, 2015:

Why MiB?

laanwj commented at 10:35 AM on December 8, 2015:

Unfortunately, the other place that logs the cache size is MiB as well: https://github.com/bitcoin/bitcoin/blob/master/src/main.cpp#L2319

-1 on changing the mempool size to be in MiB as well though - note that fees are per 1000, not per 1024, so apart from SI standards adherence kB/MB makes more sense in the context. Also per million is too granular.

MarcoFalke commented at 12:51 PM on December 8, 2015:

I thought we were parsing the maxmempool size in MB so it should be changed to kB in the other places instead.
morcos commented at 9:14 PM on December 8, 2015: member

Let's just make an executive decision on what places in the code will use MB and what will use MiB. And then we can move towards that even if not everything is correct now. Seems a bit silly to me to have two different prefix types sitting right next to each other, but if that's what we decide for some reason, then fine, but at least we'll have our justification and we won't have this debate again. I don't care what we choose, just would like to know what I should use in the future.

As for scale, personally I think on the order of MB makes more sense b/c cache and mempools are typical on the order of 100 MB. So 213231 is harder to read than 213.2 if you ask me.
MarcoFalke commented at 9:29 PM on December 8, 2015: member

executive decision on what places in the code will use MB and what will use MiB

According to #6981 we are using MB, kB etc. As 0.12 is not yet released everyone is welcome to change newly introduced MiB, kiB (like maxuploadtarget) to SI units.
morcos commented at 9:42 PM on December 8, 2015: member

oh great, sorry i missed that.
laanwj commented at 11:30 AM on December 10, 2015: member

The preference is SI units everywhere, so MB, GB, kB which are powers of 1000. To deviate from that at least have a good reason (not "I like powers of two better").

Which power of 1000 should be used depends on the quantity in question. For reporting of memory sizes, I think usually kB is the best choice not MB, as to be able to see small changes. Disk sizes should be MB or GB.
morcos commented at 4:16 PM on December 10, 2015: member

Then i suggest we use kB for both mempool and cache size in this line regardless of the fact that the cache size is powers of 2 other places.. we can fix that later if needed... lets move towards consistency
MarcoFalke commented at 4:58 PM on December 10, 2015: member

we can fix that later

This is already a cleanup PR in some sense so no need to split it into two.
MarcoFalke commented at 4:58 PM on December 10, 2015: member

@pstratem Are you still working on this?
pstratem closed this on Dec 13, 2015
DrahtBot locked this on Sep 8, 2021

Contributors

Labels