From 96f6fa44ad79349459f51547170ba17e789f6f80 Mon Sep 17 00:00:00 2001 From: Dieter Mayer Date: Mon, 27 Jul 2026 09:57:17 +0200 Subject: [PATCH] Replace hash.keys().contains(k) with hash.contains(k) (35 call sites) QHash/QMap::keys() allocates a list of every key on each call, then contains() searches it linearly - an accidental O(n) plus allocation where a direct O(1) lookup was meant. 35 occurrences across 7 files, found while profiling project load times (context: #553/#560). The hot one is ElementPictureFactory::getPictures(), which runs once per element instance on project load: on the 3399 KiB example project (191 instances, 129 cache hits) the keys() detour cost 45 ms of the 1.34 s total - measured, not estimated; the fix reproducibly shaves ~35-45 ms off that load. The remaining call sites are UI paths (search&replace, dynamic text model, undo commands) where the waste scales with selection/model size. No behavior change: for QHash/QMap, keys().contains(k) and contains(k) are equivalent by definition. (cherry picked from commit 0a7f8f072fa68de7c01a9fc134a4bc8e16d62062) --- .../ui/searchandreplacewidget.cpp | 34 +++++++++---------- sources/factory/elementpicturefactory.cpp | 2 +- sources/ui/configpage/projectconfigpages.cpp | 6 ++-- sources/ui/dynamicelementtextmodel.cpp | 18 +++++----- sources/undocommand/addelementtextcommand.cpp | 2 +- .../changeelementinformationcommand.cpp | 2 +- .../deleteqgraphicsitemcommand.cpp | 6 ++-- 7 files changed, 35 insertions(+), 35 deletions(-) diff --git a/sources/SearchAndReplace/ui/searchandreplacewidget.cpp b/sources/SearchAndReplace/ui/searchandreplacewidget.cpp index 747a873ea..7f76d33f9 100644 --- a/sources/SearchAndReplace/ui/searchandreplacewidget.cpp +++ b/sources/SearchAndReplace/ui/searchandreplacewidget.cpp @@ -1,4 +1,4 @@ -/* +/* Copyright 2006-2026 The QElectroTech Team This file is part of QElectroTech. @@ -475,7 +475,7 @@ void SearchAndReplaceWidget::setUpConenctions() connect(ui->m_tree_widget, &QTreeWidget::customContextMenuRequested, [this](const QPoint &pos) { - if (m_diagram_hash.keys().contains(ui->m_tree_widget->currentItem())) + if (m_diagram_hash.contains(ui->m_tree_widget->currentItem())) { QMenu *menu = new QMenu(ui->m_tree_widget); menu->addAction(m_select_elements); @@ -951,28 +951,28 @@ void SearchAndReplaceWidget::on_m_tree_widget_itemDoubleClicked( { Q_UNUSED(column) - if (m_diagram_hash.keys().contains(item)) + if (m_diagram_hash.contains(item)) { QPointer diagram = m_diagram_hash.value(item); if(diagram) { diagram.data()->showMe(); } } - else if (m_element_hash.keys().contains(item)) + else if (m_element_hash.contains(item)) { QPointer elmt = m_element_hash.value(item); if (elmt && elmt->diagram()) { elmt.data()->diagram()->showMe(); } } - else if (m_text_hash.keys().contains(item)) + else if (m_text_hash.contains(item)) { QPointer text = m_text_hash.value(item); if (text && text->diagram()) { text.data()->diagram()->showMe(); } } - else if (m_conductor_hash.keys().contains(item)) + else if (m_conductor_hash.contains(item)) { QPointer cond = m_conductor_hash.value(item); if (cond && cond->diagram()) { @@ -1013,7 +1013,7 @@ void SearchAndReplaceWidget::on_m_tree_widget_currentItemChanged( m_last_selected.data()->setSelected(false); } - if (m_element_hash.keys().contains(current)) + if (m_element_hash.contains(current)) { QPointer elmt = m_element_hash.value(current); if (elmt) @@ -1022,7 +1022,7 @@ void SearchAndReplaceWidget::on_m_tree_widget_currentItemChanged( elmt.data()->setHighlighted(true); } } - else if (m_text_hash.keys().contains(current)) + else if (m_text_hash.contains(current)) { QPointer text = m_text_hash.value(current); if (text) @@ -1031,7 +1031,7 @@ void SearchAndReplaceWidget::on_m_tree_widget_currentItemChanged( m_last_selected = text; } } - else if (m_conductor_hash.keys().contains(current)) + else if (m_conductor_hash.contains(current)) { QPointer cond = m_conductor_hash.value(current); if (cond) @@ -1144,7 +1144,7 @@ void SearchAndReplaceWidget::on_m_replace_pb_clicked() && qtwi->checkState(0) == Qt::Checked) { if (ui->m_folio_pb->text().endsWith(tr(" [édité]")) && - m_diagram_hash.keys().contains(qtwi)) + m_diagram_hash.contains(qtwi)) { QPointer d = m_diagram_hash.value(qtwi); if (d) { @@ -1152,7 +1152,7 @@ void SearchAndReplaceWidget::on_m_replace_pb_clicked() } } else if (ui->m_element_pb->text().endsWith(tr(" [édité]")) && - m_element_hash.keys().contains(qtwi)) + m_element_hash.contains(qtwi)) { QPointer e = m_element_hash.value(qtwi); if (e) { @@ -1160,7 +1160,7 @@ void SearchAndReplaceWidget::on_m_replace_pb_clicked() } } else if (!ui->m_replace_le->text().isEmpty() && - m_text_hash.keys().contains(qtwi)) + m_text_hash.contains(qtwi)) { m_worker.m_indi_text = ui->m_replace_le->text(); QPointer t = @@ -1171,7 +1171,7 @@ void SearchAndReplaceWidget::on_m_replace_pb_clicked() } else if (ui->m_conductor_pb->text().endsWith(tr(" [édité]")) && - m_conductor_hash.keys().contains(qtwi)) + m_conductor_hash.contains(qtwi)) { QPointer c = m_conductor_hash.value(qtwi); if (c) { @@ -1187,7 +1187,7 @@ void SearchAndReplaceWidget::on_m_replace_pb_clicked() QList tl; QList cl; - if (m_diagram_hash.keys().contains(qtwi)) + if (m_diagram_hash.contains(qtwi)) { QPointer d = m_diagram_hash.value(qtwi); @@ -1195,7 +1195,7 @@ void SearchAndReplaceWidget::on_m_replace_pb_clicked() dl.append(d.data()); } } - else if (m_element_hash.keys().contains(qtwi)) + else if (m_element_hash.contains(qtwi)) { QPointer e = m_element_hash.value(qtwi); @@ -1203,7 +1203,7 @@ void SearchAndReplaceWidget::on_m_replace_pb_clicked() el.append(e.data()); } } - else if (m_text_hash.keys().contains(qtwi)) + else if (m_text_hash.contains(qtwi)) { QPointer t = m_text_hash.value(qtwi); @@ -1211,7 +1211,7 @@ void SearchAndReplaceWidget::on_m_replace_pb_clicked() tl.append(t.data()); } } - else if (m_conductor_hash.keys().contains(qtwi)) + else if (m_conductor_hash.contains(qtwi)) { QPointer c = m_conductor_hash.value(qtwi); diff --git a/sources/factory/elementpicturefactory.cpp b/sources/factory/elementpicturefactory.cpp index 1f8af820b..a2a6655b7 100644 --- a/sources/factory/elementpicturefactory.cpp +++ b/sources/factory/elementpicturefactory.cpp @@ -54,7 +54,7 @@ void ElementPictureFactory::getPictures(const ElementsLocation &location, QPictu return; } - if(m_pictures_H.keys().contains(uuid)) + if(m_pictures_H.contains(uuid)) { picture = m_pictures_H.value(uuid); low_picture = m_low_pictures_H.value(uuid); diff --git a/sources/ui/configpage/projectconfigpages.cpp b/sources/ui/configpage/projectconfigpages.cpp index 89d02bb43..422e27109 100644 --- a/sources/ui/configpage/projectconfigpages.cpp +++ b/sources/ui/configpage/projectconfigpages.cpp @@ -419,7 +419,7 @@ void ProjectAutoNumConfigPage::saveContextElement() m_saw_element->contextComboBox()->addItem(tr("Sans nom")); } // If the text isn't yet to the autonum of the project, add this new item to the combo box. - else if ( !m_project -> elementAutoNum().keys().contains( m_saw_element->contextComboBox()->currentText())) + else if ( !m_project -> elementAutoNum().contains( m_saw_element->contextComboBox()->currentText())) { m_project->addElementAutoNum(m_saw_element->contextComboBox()->currentText(), m_saw_element->toNumContext()); m_project->setCurrrentElementAutonum(m_saw_element->contextComboBox()->currentText()); @@ -461,7 +461,7 @@ void ProjectAutoNumConfigPage::saveContextConductor() m_saw_conductor->contextComboBox()-> addItem(tr("Sans nom")); } // If the text isn't yet to the autonum of the project, add this new item to the combo box. - else if ( !m_project -> conductorAutoNum().keys().contains( m_saw_conductor->contextComboBox()->currentText())) + else if ( !m_project -> conductorAutoNum().contains( m_saw_conductor->contextComboBox()->currentText())) { project()->addConductorAutoNum(m_saw_conductor->contextComboBox()->currentText(), m_saw_conductor->toNumContext()); project()->setCurrentConductorAutoNum(m_saw_conductor->contextComboBox()->currentText()); @@ -489,7 +489,7 @@ void ProjectAutoNumConfigPage::saveContextFolio() m_saw_folio->contextComboBox() -> addItem(tr("Sans nom")); } // If the text isn't yet to the autonum of the project, add this new item to the combo box. - else if ( !m_project -> folioAutoNum().keys().contains( m_saw_folio->contextComboBox()->currentText())) { + else if ( !m_project -> folioAutoNum().contains( m_saw_folio->contextComboBox()->currentText())) { project()->addFolioAutoNum(m_saw_folio->contextComboBox()->currentText(), m_saw_folio->toNumContext()); m_saw_folio->contextComboBox() -> addItem(m_saw_folio->contextComboBox()->currentText()); } diff --git a/sources/ui/dynamicelementtextmodel.cpp b/sources/ui/dynamicelementtextmodel.cpp index 8e4c91868..f33128729 100644 --- a/sources/ui/dynamicelementtextmodel.cpp +++ b/sources/ui/dynamicelementtextmodel.cpp @@ -130,7 +130,7 @@ QList DynamicElementTextModel::itemsForText( { QList qsi_list; - if(m_texts_list.keys().contains(deti)) + if(m_texts_list.contains(deti)) return qsi_list; QStandardItem *qsi = new QStandardItem(deti->toPlainText()); @@ -718,7 +718,7 @@ QUndoCommand *DynamicElementTextModel::undoForEditedGroup( */ void DynamicElementTextModel::addGroup(ElementTextItemGroup *group) { - if(m_groups_list.keys().contains(group)) + if(m_groups_list.contains(group)) return; //Group @@ -869,7 +869,7 @@ void DynamicElementTextModel::addGroup(ElementTextItemGroup *group) */ void DynamicElementTextModel::removeGroup(ElementTextItemGroup *group) { - if(m_groups_list.keys().contains(group)) + if(m_groups_list.contains(group)) { QModelIndex group_index = m_groups_list.value(group)->index(); this->removeRow(group_index.row(), group_index.parent()); @@ -896,7 +896,7 @@ void DynamicElementTextModel::removeTextFromGroup(DynamicElementTextItem *deti, { Q_UNUSED(group) - if(m_texts_list.keys().contains(deti)) + if(m_texts_list.contains(deti)) { QStandardItem *text_item = m_texts_list.value(deti); QModelIndex text_index = indexFromItem(text_item); @@ -961,7 +961,7 @@ ElementTextItemGroup *DynamicElementTextModel::groupFromItem( QModelIndex DynamicElementTextModel::indexFromGroup( ElementTextItemGroup *group) const { - if(m_groups_list.keys().contains(group)) + if(m_groups_list.contains(group)) return m_groups_list.value(group)->index(); else return QModelIndex(); @@ -1371,7 +1371,7 @@ void DynamicElementTextModel::setConnection(DynamicElementTextItem *deti, bool s { if(set) { - if(m_hash_text_connect.keys().contains(deti)) + if(m_hash_text_connect.contains(deti)) return; QList connection_list; @@ -1390,7 +1390,7 @@ void DynamicElementTextModel::setConnection(DynamicElementTextItem *deti, bool s } else { - if(!m_hash_text_connect.keys().contains(deti)) + if(!m_hash_text_connect.contains(deti)) return; for (const QMetaObject::Connection& con : m_hash_text_connect.value(deti)) @@ -1412,7 +1412,7 @@ void DynamicElementTextModel::setConnection(ElementTextItemGroup *group, bool se { if(set) { - if(m_hash_group_connect.keys().contains(group)) + if(m_hash_group_connect.contains(group)) return; QList connection_list; @@ -1429,7 +1429,7 @@ void DynamicElementTextModel::setConnection(ElementTextItemGroup *group, bool se } else { - if(!m_hash_group_connect.keys().contains(group)) + if(!m_hash_group_connect.contains(group)) return; for (const QMetaObject::Connection& con : m_hash_group_connect.value(group)) diff --git a/sources/undocommand/addelementtextcommand.cpp b/sources/undocommand/addelementtextcommand.cpp index da8662f71..ea005935c 100644 --- a/sources/undocommand/addelementtextcommand.cpp +++ b/sources/undocommand/addelementtextcommand.cpp @@ -447,7 +447,7 @@ void AlignmentTextsGroupCommand::undo() { for(DynamicElementTextItem *deti : m_group.data()->texts()) { - if(m_texts_pos.keys().contains(deti)) + if(m_texts_pos.contains(deti)) deti->setPos(m_texts_pos.value(deti)); } } diff --git a/sources/undocommand/changeelementinformationcommand.cpp b/sources/undocommand/changeelementinformationcommand.cpp index d06566e0a..ffc417dc0 100644 --- a/sources/undocommand/changeelementinformationcommand.cpp +++ b/sources/undocommand/changeelementinformationcommand.cpp @@ -61,7 +61,7 @@ bool ChangeElementInformationCommand::mergeWith(const QUndoCommand *other) if (m_map.size() == other_undo->m_map.size()) { for (auto key : other_undo->m_map.keys()) { - if (!m_map.keys().contains(key)) { + if (!m_map.contains(key)) { return false; } } diff --git a/sources/undocommand/deleteqgraphicsitemcommand.cpp b/sources/undocommand/deleteqgraphicsitemcommand.cpp index 8f9fd9542..a2db0baf8 100644 --- a/sources/undocommand/deleteqgraphicsitemcommand.cpp +++ b/sources/undocommand/deleteqgraphicsitemcommand.cpp @@ -86,7 +86,7 @@ DeleteQGraphicsItemCommand::DeleteQGraphicsItemCommand( for (auto table : m_removed_contents.m_tables) { //Table is already managed, jump to next loop - if (m_table_scene_hash.keys().contains(table)) + if (m_table_scene_hash.contains(table)) continue; auto first_table = table; //The first table if the table is linked to another @@ -290,9 +290,9 @@ void DeleteQGraphicsItemCommand::undo() for(DynamicElementTextItem *deti : m_removed_contents.m_element_texts) { - if(m_elmt_text_hash.keys().contains(deti)) + if(m_elmt_text_hash.contains(deti)) m_elmt_text_hash.value(deti)->addDynamicTextItem(deti); - else if (m_grp_texts_hash.keys().contains(deti)) + else if (m_grp_texts_hash.contains(deti)) { Element *elmt = m_grp_texts_hash.value(deti)->parentElement(); elmt->addDynamicTextItem(deti);