Having the mempool size in the debug window can be useful. This PR also moves the "open debug log" button to the right side of the debug window.
The stats gets updated over the same TryLock that is used for the bandwidth graph (no additional blocking is required).
It will ~look like this: <img width="853" alt="bildschirmfoto 2015-11-10 um 13 50 05" src="https://cloud.githubusercontent.com/assets/178464/11062770/f7f6ac4e-87b1-11e5-892f-aa51afe86275.png">
352 | </item> 353 | <item row="15" column="0"> 354 | - <widget class="QLabel" name="labelDebugLogfile"> 355 | + <widget class="QLabel" name="label_3"> 356 | + <property name="text"> 357 | + <string>Current amount of transactions</string>
Current number of transactions
382 | - <widget class="QPushButton" name="openDebugLogfileButton"> 383 | - <property name="toolTip"> 384 | - <string>Open the Bitcoin Core debug log file from the current data directory. This can take a few seconds for large log files.</string> 385 | + <widget class="QLabel" name="label_2"> 386 | + <property name="text"> 387 | + <string>Dynamic memory usage</string>
I'd leave out 'dynamic'
Concept ACK
76 | @@ -77,6 +77,8 @@ public Q_SLOTS: 77 | void setNumConnections(int count); 78 | /** Set number of blocks and last block date shown in the UI */ 79 | void setNumBlocks(int count, const QDateTime& blockDate); 80 | + /** Set size (amount of transactions and memory usage) of the mempool in the UI */
amount -> number
Can you please change Mempool in the above screenshot to Memory pool? Nice. utACK. Will test later.
I'd like to see also the graph of the memory pool - similar to the Network Traffic tab...
@paveljanik: thanks for the review. Yes. The mempool chart is also on my list of things i'd like to do (... and that list is already very large).
conceptACK
Nit's addressed with two squashme commits.
OS X screenshot that includes the latest changes.
Random suggestion: under "Block chain", put a " Database cache siE" entry that is pcoinsTip->GetDynamicUsage()" ?
@sipa: good point. But because of the window height (I try not to enlarge), maybe the database cache size fits better in a new "limits" (or "usage") screen/graph. There i could imaging a mempool size/memusage chart as well as the database-cache (chart and current value).
Fair enough, keep the pull request as-is then.
530 | + ui->mempoolNumberTxs->setText(QString::number(numberOfTxs)); 531 | + 532 | + if (dynUsage < 1024*1024) 533 | + ui->mempoolSize->setText(QString::number(dynUsage/1024.0, 'f', 2) + " KB"); 534 | + else 535 | + ui->mempoolSize->setText(QString::number(dynUsage/1024.0/1024.0, 'f', 2) + " MB");
I don't like all those MB, MiB inconsistencies. At least make it consistent with the parameter which sets the mempool size. (Which is not MiB iirc)
We should be using MB (1e6 bytes) unless there is a convincing reason to use MiB (2^20 bytes).
Some people refer to 2^20 bytes as "MB", so at least MiB is clear what it does.
But it's used less commonly, so MB is fine as well. Just be consistent, which is what the NIT is complaining about. You can't parse the command line in MB (1e6) and then display MB (10^20)
In bitcoin core we stick to SI units and use MB, which is defined as 1000000, not 1024*1024, MiB, etc if we do that that's a mistake.
Technically, B in SI means bel. B as byte is defined by IEEE 1541 (which also defined b to mean bit).
Looks good, Concept ACK.
Compile log compared to the master:
+qt/forms/debugwindow.ui: Warning: The name 'label_10' (QLabel) is already in use, defaulting to 'label_101'.
+qt/forms/debugwindow.ui: Warning: The name 'label_3' (QLabel) is already in use, defaulting to 'label_31'.
+qt/forms/debugwindow.ui: Warning: The name 'label_11' (QLabel) is already in use, defaulting to 'label_111'.
+qt/forms/debugwindow.ui: Warning: The name 'label_2' (QLabel) is already in use, defaulting to 'label_21'.
+qt/forms/debugwindow.ui: Warning: The name 'verticalLayout_4' (QVBoxLayout) is already in use, defaulting to 'verticalLayout_41'.
+qt/forms/debugwindow.ui: Warning: The name 'label_21' (QLabel) is already in use, defaulting to 'label_211'.
@paveljanik: these warning are well known and they are unrelated to this PR. Have also plans to fix the qt id/name conflicts (warnings) soon.
Switched to MB / SI units (1000^2 for MB).
@jonasschnelli I compared the log of the master with the log of the master with this PR. I do not see such warnings in the master builds... I have not investigated from where they come though.
373 | + <set>Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse</set> 374 | + </property> 375 | + </widget> 376 | + </item> 377 | + <item row="8" column="0"> 378 | + <widget class="QLabel" name="label_11">
As pointed out by @paveljanik label_11 seems already in use: C.f. $ grep -r 'label_11' src/qt/
Generally speaking: Is it encouraged to use (name)_i++ for references? I'd prefer something like label_memp_whatever.
Right. Agree with that. It was already named label_11 before this PR. The diff looks a bit strange. But i have fixed this now.
Right. There where QT name/id conflicts (some introduced over this PR), although, Qt auto-increments conflicting IDs and because we don't access them through the code, it wouldn't be a problem. But it's ugly, agreed.
Added a squashme-commit with a fix.
Here are the binaries if someone likes to test this: https://bitcoin.jonasschnelli.ch/pulls/6979/
Memory usage is continuously updated here, but always ends with .00 KB (on testnet). I have never seen any decimal number there. Do we need these two decimal zeroes?
@paveljanik: if the memory usage is grater than 1000000 bytes it will use "MB" and there the precision of two decimal places make sense IMO.
Fixed the size_t to float conversion. I accidentally dropped the 1000.0/1000000.0 in the arithmetic.
ACK
I was wondering how you get the first screenshot :+1:
I was wondering how you get the first screenshot
I think somewhere the .0 was lost during implementation of the KB/MB switch logic (and the screenshots was done before that). But this does not explain @fanquake 's screen. ?! Maybe on osx QString::number() does somehow evaluate the division always as float?
I do not think so (division is evaluated first). Maybe his screenshot is from the same code from which you did the screenshot.
88 | @@ -88,6 +89,16 @@ QDateTime ClientModel::getLastBlockDate() const 89 | return QDateTime::fromTime_t(Params().GenesisBlock().GetBlockTime()); // Genesis block's time of current network 90 | } 91 | 92 | +size_t ClientModel::getMempoolSize() const
Nit: Isn't this long?
Thanks! Right, .. the map's size() itself is size_t, but the CTxMempool uses long size() in its interface.
Fixed.
utACK c197798
Teseted ACK c197798. Code looks clean.
Tested ACK
Milestone
0.12.0