From 8da694c9c38e8562ef2b0b198ef9f5aa09c8ea37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Mon, 16 Jun 2025 18:46:33 +0200 Subject: [PATCH] history_list: pre-compute sort keys, don't launder getting them through get_data_for_role() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit endInsertRows() triggers a sort, the history sort/filter proxy model then launders the comparison requests through get_data_for_role() with a custom key, which is then especially looked up in there, but custom-flattened in the proxy model. This is OO hell Measuring refresh() from after if transactions == self.transactions: return to the final return, sorting the wallet from #9958/#6625 takes ~3.5s and 132348 get_data_for_role() calls Beside that get_data_for_role() is called maybe 10 times per on-hover? It's immaterial Thus: compute the sorting keys when constructing the HistoryNode and use them directly in the comparison. This decouples get_data_for_role() from the sorter and speeds the sort up by ~2x, now being mostly made up of the python interpreter overhead(?) for upcalls to lessThan() A dict wins with a list for the lookup a tiny bit (and is 100x better code-wise): 1.896097298245877s list 1.840533264912665s list 1.757185084745288s list 1.8990754359401762s list 1.914668960031122s list 1.9349112827330828s list 2.432649422902614s list 1.929884395096451s list 1.9610795709304512s list 1.8694845158606768s list 1.9600030612200499s list 1.9199693519622087s dict 2.0466488380916417s dict 1.8510140180587769s dict 1.8978681536391377s dict 2.0079748439602554s dict 1.9111531740054488s dict 1.9525738609954715s dict 1.850804285146296s dict 1.860573346260935s dict 1.95173170696944s dict 1.8481200002133846s dict Benchmark 1: dict Time (mean ± σ): 1918.039234 ms ± 63.688609 ms Range (min … max): 1848.120000 ms … 2046.648838 ms 11 runs Benchmark 2: list Time (mean ± σ): 1945.052027 ms ± 164.019588 ms Range (min … max): 1757.185085 ms … 2432.649423 ms 11 runs Summary 'dict' ran 1.014084 ± 0.090082 times faster than 'list' --- electrum/gui/qt/history_list.py | 99 ++++++++++++++++++--------------- 1 file changed, 53 insertions(+), 46 deletions(-) diff --git a/electrum/gui/qt/history_list.py b/electrum/gui/qt/history_list.py index 4b1b661fb..67061348d 100644 --- a/electrum/gui/qt/history_list.py +++ b/electrum/gui/qt/history_list.py @@ -75,36 +75,70 @@ TX_ICONS = [ ] -ROLE_SORT_ORDER = Qt.ItemDataRole.UserRole + 1000 - - class HistorySortModel(QSortFilterProxyModel): + + def data_for(self, index: QModelIndex): + col = index.column() + if col == HistoryColumns.STATUS: + # respect sort order of self.transactions (wallet.get_full_history) + return -index.row() + else: + node = index.internalPointer() + return node.sort_keys[col] + def lessThan(self, source_left: QModelIndex, source_right: QModelIndex): - item1 = self.sourceModel().data(source_left, ROLE_SORT_ORDER) - item2 = self.sourceModel().data(source_right, ROLE_SORT_ORDER) - if item1 is None or item2 is None: - raise Exception(f'UserRole not set for column {source_left.column()}') - v1 = item1.value() - v2 = item2.value() - if v1 is None or isinstance(v1, Decimal) and v1.is_nan(): v1 = -float("inf") - if v2 is None or isinstance(v2, Decimal) and v2.is_nan(): v2 = -float("inf") - try: - return v1 < v2 - except Exception: - return False + return self.data_for(source_left) < self.data_for(source_right) def get_item_key(tx_item): return tx_item.get('txid') or tx_item['payment_hash'] +def flatten_sort_key(v): + if v is None or isinstance(v, Decimal) and v.is_nan(): + return -float("inf") + else: + return v + class HistoryNode(CustomNode): model: 'HistoryModel' + def __init__(self, model: 'CustomModel', tx_item): + super().__init__(model, tx_item) + + if tx_item is None: + tx_item = {} + is_lightning = tx_item.get('lightning', False) + short_id = "" + if not is_lightning: + txpos_in_block = tx_item.get('txpos_in_block', -1) + if txpos_in_block >= 0: + short_id = f"{tx_item['height']}x{txpos_in_block}" + self.sort_keys = { + HistoryColumns.DESCRIPTION: flatten_sort_key( + tx_item.get('label')), + HistoryColumns.AMOUNT: flatten_sort_key( + (tx_item['bc_value'].value if 'bc_value' in tx_item else 0)\ + + (tx_item['ln_value'].value if 'ln_value' in tx_item else 0)), + HistoryColumns.BALANCE: 0, + HistoryColumns.FIAT_VALUE: flatten_sort_key( + tx_item['fiat_value'].value if 'fiat_value' in tx_item else None), + HistoryColumns.FIAT_ACQ_PRICE: flatten_sort_key( + tx_item['acquisition_price'].value if 'acquisition_price' in tx_item else None), + HistoryColumns.FIAT_CAP_GAINS: flatten_sort_key( + tx_item['capital_gain'].value if 'capital_gain' in tx_item else None), + HistoryColumns.TXID: flatten_sort_key( + tx_item.get('txid') if not is_lightning else None), + HistoryColumns.SHORT_ID: + short_id, + } + + def set_balance(self, balance): + self._data['balance'] = Satoshis(balance) + self.sort_keys[HistoryColumns.BALANCE] = balance + def get_data_for_role(self, index: QModelIndex, role: Qt.ItemDataRole) -> QVariant: - # note: this method is performance-critical. - # it is called a lot, and so must run extremely fast. assert index.isValid() col = index.column() window = self.model.window @@ -115,7 +149,6 @@ class HistoryNode(CustomNode): # and the group does not have an onchain tx is_lightning = True timestamp = tx_item['timestamp'] - short_id = None if is_lightning: status = 0 if timestamp is None: @@ -124,10 +157,6 @@ class HistoryNode(CustomNode): status_str = format_time(int(timestamp)) else: tx_hash = tx_item['txid'] - if col == HistoryColumns.SHORT_ID: - txpos_in_block = tx_item.get('txpos_in_block', -1) - if txpos_in_block >= 0: - short_id = f"{tx_item['height']}x{txpos_in_block}" conf = tx_item['confirmations'] try: status, status_str = self.model.tx_status_cache[tx_hash] @@ -135,28 +164,6 @@ class HistoryNode(CustomNode): tx_mined_info = self.model._tx_mined_info_from_tx_item(tx_item) status, status_str = window.wallet.get_tx_status(tx_hash, tx_mined_info) - if role == ROLE_SORT_ORDER: - d = { - HistoryColumns.STATUS: - # respect sort order of self.transactions (wallet.get_full_history) - -index.row(), - HistoryColumns.DESCRIPTION: - tx_item['label'] if 'label' in tx_item else None, - HistoryColumns.AMOUNT: - (tx_item['bc_value'].value if 'bc_value' in tx_item else 0)\ - + (tx_item['ln_value'].value if 'ln_value' in tx_item else 0), - HistoryColumns.BALANCE: - (tx_item['balance'].value if 'balance' in tx_item else 0), - HistoryColumns.FIAT_VALUE: - tx_item['fiat_value'].value if 'fiat_value' in tx_item else None, - HistoryColumns.FIAT_ACQ_PRICE: - tx_item['acquisition_price'].value if 'acquisition_price' in tx_item else None, - HistoryColumns.FIAT_CAP_GAINS: - tx_item['capital_gain'].value if 'capital_gain' in tx_item else None, - HistoryColumns.TXID: tx_hash if not is_lightning else None, - HistoryColumns.SHORT_ID: short_id, - } - return QVariant(d[col]) if role == MyTreeView.ROLE_EDIT_KEY: return QVariant(get_item_key(tx_item)) if role not in (Qt.ItemDataRole.DisplayRole, Qt.ItemDataRole.EditRole, MyTreeView.ROLE_CLIPBOARD_DATA): @@ -227,7 +234,7 @@ class HistoryNode(CustomNode): elif col == HistoryColumns.TXID: return QVariant(tx_hash) if not is_lightning else QVariant('') elif col == HistoryColumns.SHORT_ID: - return QVariant(short_id or "") + return QVariant(self.sort_keys[HistoryColumns.SHORT_ID]) return QVariant() @@ -316,7 +323,7 @@ class HistoryModel(CustomModel, Logger): balance = 0 for node in self._root._children: balance += node._data['value'].value - node._data['balance'] = Satoshis(balance) + node.set_balance(balance) # update tx_status_cache (before endInsertRows() triggers get_data_for_role() calls) self.tx_status_cache.clear()