From 316f6b17bd3f6a444c71a80f9bbcf5fc1a00968c Mon Sep 17 00:00:00 2001 From: jakub-w Date: Thu, 9 May 2019 15:36:04 +0200 Subject: [PATCH 1/4] Improved performance of adding rows and columns to the WTable. The issue was the calling of WTableRow::rowNum() for every added cell, which iterated over the whole table. The methods WTableRow::createCell() and WTableRow::insertColumn() got the new row argument defaulting to -1 that WTable can use when creating new elements with WTable::insertRow() and WTable::insertColumn(). These methods are called by WTable::expand() so the performance improvement affects every operation that expands the table. The performance of populating a new table row by row is increased with this commit from O(n^2+n) to O(n). --- src/Wt/WTable.C | 4 ++-- src/Wt/WTableRow.C | 18 +++++++++++------- src/Wt/WTableRow.h | 6 +++--- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/Wt/WTable.C b/src/Wt/WTable.C index 4d10062c6b..826efb0526 100644 --- a/src/Wt/WTable.C +++ b/src/Wt/WTable.C @@ -103,7 +103,7 @@ WTableRow* WTable::insertRow(int row, std::unique_ptr tableRow) widgetAdded(cell.get()); } rows_.insert(rows_.begin() + row, std::move(tableRow)); - rows_[row].get()->expand(columnCount()); + rows_[row].get()->expand(columnCount(), row); repaint(RepaintFlag::SizeAffected); return rows_[row].get(); @@ -113,7 +113,7 @@ WTableColumn* WTable::insertColumn(int column, std::unique_ptr tableColumn) { for (unsigned i = 0; i < rows_.size(); ++i) - rows_[i]->insertColumn(column); + rows_[i]->insertColumn(column, i); if ((unsigned)column <= columns_.size()) { if (!tableColumn){ diff --git a/src/Wt/WTableRow.C b/src/Wt/WTableRow.C index d6981618cf..21077cd159 100644 --- a/src/Wt/WTableRow.C +++ b/src/Wt/WTableRow.C @@ -26,19 +26,23 @@ WTableRow::WTableRow() WTableRow::~WTableRow() { } -std::unique_ptr WTableRow::createCell(int column) +std::unique_ptr WTableRow::createCell(int column, int row) { - if (table_) - return table_->createCell(rowNum(), column); + if (table_) { + if (-1 == row) { + row = rowNum(); + } + return table_->createCell(row, column); + } else return std::unique_ptr(new WTableCell()); } -void WTableRow::expand(int numCells) +void WTableRow::expand(int numCells, int row) { int cursize = cells_.size(); for (int col = cursize; col < numCells; ++col) { - cells_.push_back(createCell(col)); + cells_.push_back(createCell(col, row)); WTableCell *cell = cells_.back().get(); if (table_) table_->widgetAdded(cell); @@ -47,9 +51,9 @@ void WTableRow::expand(int numCells) } } -void WTableRow::insertColumn(int column) +void WTableRow::insertColumn(int column, int row) { - cells_.insert(cells_.begin() + column, createCell(column)); + cells_.insert(cells_.begin() + column, createCell(column, row)); WTableCell *cell = cells_[column].get(); if (table_) table_->widgetAdded(cell); diff --git a/src/Wt/WTableRow.h b/src/Wt/WTableRow.h index e8884553dc..171d99ed72 100644 --- a/src/Wt/WTableRow.h +++ b/src/Wt/WTableRow.h @@ -136,10 +136,10 @@ class WT_API WTableRow : public WObject virtual const std::string id() const override; protected: - virtual std::unique_ptr createCell(int column); + virtual std::unique_ptr createCell(int column, int row = -1); private: - void expand(int numCells); + void expand(int numCells, int row = -1); WTable *table_; std::vector > cells_; @@ -151,7 +151,7 @@ class WT_API WTableRow : public WObject void updateDom(DomElement& element, bool all); void setTable(WTable *table); - void insertColumn(int column); + void insertColumn(int column, int row = -1); std::unique_ptr removeColumn(int column); void undoHide(); From 72bb2020dda7c6e6508c55792f27ab0a87d143e2 Mon Sep 17 00:00:00 2001 From: jakub-w Date: Sat, 5 Oct 2019 09:32:57 +0200 Subject: [PATCH 2/4] Revert "Improved performance of adding rows and columns to the WTable." This reverts commit 316f6b17bd3f6a444c71a80f9bbcf5fc1a00968c. --- src/Wt/WTable.C | 4 ++-- src/Wt/WTableRow.C | 18 +++++++----------- src/Wt/WTableRow.h | 6 +++--- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/Wt/WTable.C b/src/Wt/WTable.C index 826efb0526..4d10062c6b 100644 --- a/src/Wt/WTable.C +++ b/src/Wt/WTable.C @@ -103,7 +103,7 @@ WTableRow* WTable::insertRow(int row, std::unique_ptr tableRow) widgetAdded(cell.get()); } rows_.insert(rows_.begin() + row, std::move(tableRow)); - rows_[row].get()->expand(columnCount(), row); + rows_[row].get()->expand(columnCount()); repaint(RepaintFlag::SizeAffected); return rows_[row].get(); @@ -113,7 +113,7 @@ WTableColumn* WTable::insertColumn(int column, std::unique_ptr tableColumn) { for (unsigned i = 0; i < rows_.size(); ++i) - rows_[i]->insertColumn(column, i); + rows_[i]->insertColumn(column); if ((unsigned)column <= columns_.size()) { if (!tableColumn){ diff --git a/src/Wt/WTableRow.C b/src/Wt/WTableRow.C index 21077cd159..d6981618cf 100644 --- a/src/Wt/WTableRow.C +++ b/src/Wt/WTableRow.C @@ -26,23 +26,19 @@ WTableRow::WTableRow() WTableRow::~WTableRow() { } -std::unique_ptr WTableRow::createCell(int column, int row) +std::unique_ptr WTableRow::createCell(int column) { - if (table_) { - if (-1 == row) { - row = rowNum(); - } - return table_->createCell(row, column); - } + if (table_) + return table_->createCell(rowNum(), column); else return std::unique_ptr(new WTableCell()); } -void WTableRow::expand(int numCells, int row) +void WTableRow::expand(int numCells) { int cursize = cells_.size(); for (int col = cursize; col < numCells; ++col) { - cells_.push_back(createCell(col, row)); + cells_.push_back(createCell(col)); WTableCell *cell = cells_.back().get(); if (table_) table_->widgetAdded(cell); @@ -51,9 +47,9 @@ void WTableRow::expand(int numCells, int row) } } -void WTableRow::insertColumn(int column, int row) +void WTableRow::insertColumn(int column) { - cells_.insert(cells_.begin() + column, createCell(column, row)); + cells_.insert(cells_.begin() + column, createCell(column)); WTableCell *cell = cells_[column].get(); if (table_) table_->widgetAdded(cell); diff --git a/src/Wt/WTableRow.h b/src/Wt/WTableRow.h index 171d99ed72..e8884553dc 100644 --- a/src/Wt/WTableRow.h +++ b/src/Wt/WTableRow.h @@ -136,10 +136,10 @@ class WT_API WTableRow : public WObject virtual const std::string id() const override; protected: - virtual std::unique_ptr createCell(int column, int row = -1); + virtual std::unique_ptr createCell(int column); private: - void expand(int numCells, int row = -1); + void expand(int numCells); WTable *table_; std::vector > cells_; @@ -151,7 +151,7 @@ class WT_API WTableRow : public WObject void updateDom(DomElement& element, bool all); void setTable(WTable *table); - void insertColumn(int column, int row = -1); + void insertColumn(int column); std::unique_ptr removeColumn(int column); void undoHide(); From 989c2ad967b6cdded3eeb6ddd81b9ad53fb163c3 Mon Sep 17 00:00:00 2001 From: jakub-w Date: Sat, 5 Oct 2019 11:47:33 +0200 Subject: [PATCH 3/4] Added tests for WTable Two of them (moveRow2 and moveColumn2) do not pass because of a bug in the WTable's code, which is not adressed in this commit. That's why these ones are disabled. The tests are not that comprehensive and were created to test the correctnes of code in future commits adressing WTable's performance. --- test/CMakeLists.txt | 1 + test/widgets/WTableTest.C | 269 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+) create mode 100644 test/widgets/WTableTest.C diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 4275330af0..97dcbe5c55 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -32,6 +32,7 @@ IF(ENABLE_LIBWTTEST) utils/Base64Test.C utils/EraseWord.C utils/ParseNumber.C + widgets/WTableTest.C widgets/WSpinBoxTest.C length/WLengthTest.C color/WColorTest.C diff --git a/test/widgets/WTableTest.C b/test/widgets/WTableTest.C new file mode 100644 index 0000000000..214d135673 --- /dev/null +++ b/test/widgets/WTableTest.C @@ -0,0 +1,269 @@ +/* + * Copyright (C) 2010 Emweb bvba, Kessel-Lo, Belgium. + * + * See the LICENSE file for terms of use. + */ +#include + +#include +#include +#include + +using namespace Wt; + +namespace { +Wt::Test::WTestEnvironment environment; +Wt::WApplication testApp(environment); + +void CheckRowNumbers(WTable* table) +{ + for (auto i = 0; i < table->rowCount(); ++i) { + BOOST_REQUIRE_EQUAL(table->rowAt(i)->rowNum(), i); + } +} +void CheckColumnNumbers(WTable* table) +{ + for (auto i = 0; i < table->columnCount(); ++i) { + BOOST_REQUIRE_EQUAL(table->columnAt(i)->columnNum(), i); + } +} +} + +BOOST_AUTO_TEST_SUITE( WTableTest ) + +BOOST_AUTO_TEST_CASE( elementAt1 ) +{ + auto table = cpp14::make_unique(); + BOOST_REQUIRE_NO_THROW(table->elementAt(5, 1)); + + auto rowCount = table->rowCount(); + + BOOST_REQUIRE_EQUAL(rowCount, 6); + CheckRowNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( elementAt2 ) +{ + auto table = cpp14::make_unique(); + BOOST_REQUIRE_NO_THROW(table->elementAt(1, 5)); + + auto columnCount = table->columnCount(); + + BOOST_REQUIRE_EQUAL(columnCount, 6); + CheckColumnNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( elementAt3 ) +{ + auto table = cpp14::make_unique(); + BOOST_REQUIRE_NO_THROW(table->elementAt(5, 4)); + + auto rowCount = table->rowCount(); + auto columnCount = table->columnCount(); + + BOOST_REQUIRE_EQUAL(rowCount, 6); + BOOST_REQUIRE_EQUAL(columnCount, 5); + CheckRowNumbers(table.get()); + CheckColumnNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( rowAt ) +{ + auto table = cpp14::make_unique(); + BOOST_REQUIRE_NO_THROW(table->rowAt(5)); + + auto rowCount = table->rowCount(); + + BOOST_REQUIRE_EQUAL(rowCount, 6); + CheckRowNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( columnAt ) +{ + auto table = cpp14::make_unique(); + BOOST_REQUIRE_NO_THROW(table->columnAt(5)); + + auto columnCount = table->columnCount(); + + BOOST_REQUIRE_EQUAL(columnCount, 6); + CheckColumnNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( insertRow1 ) +{ + auto table = cpp14::make_unique(); + table->rowAt(2); + BOOST_REQUIRE_NO_THROW(table->insertRow(1)); + + auto rowCount = table->rowCount(); + + BOOST_REQUIRE_EQUAL(rowCount, 4); + CheckRowNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( insertRow2 ) +{ + auto table = cpp14::make_unique(); + table->rowAt(2); + BOOST_REQUIRE_NO_THROW(table->insertRow(3)); + + auto rowCount = table->rowCount(); + + BOOST_REQUIRE_EQUAL(rowCount, 4); + CheckRowNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( insertRow3 ) +{ + auto table = cpp14::make_unique(); + BOOST_REQUIRE_NO_THROW(table->insertRow(0)); +} + +BOOST_AUTO_TEST_CASE( insertColumn1 ) +{ + auto table = cpp14::make_unique(); + table->columnAt(2); + BOOST_REQUIRE_NO_THROW(table->insertColumn(1)); + + auto columnCount = table->columnCount(); + + BOOST_REQUIRE_EQUAL(columnCount, 4); + CheckColumnNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( insertColumn2 ) +{ + auto table = cpp14::make_unique(); + table->columnAt(2); + BOOST_REQUIRE_NO_THROW(table->insertColumn(3)); + + auto columnCount = table->columnCount(); + + BOOST_REQUIRE_EQUAL(columnCount, 4); + CheckColumnNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( insertColumn3 ) +{ + auto table = cpp14::make_unique(); + BOOST_REQUIRE_NO_THROW(table->insertColumn(0)); +} + +BOOST_AUTO_TEST_CASE( removeRow1 ) +{ + auto table = cpp14::make_unique(); + table->rowAt(5); + BOOST_REQUIRE_NO_THROW(table->removeRow(2)); + + auto rowCount = table->rowCount(); + + BOOST_REQUIRE_EQUAL(rowCount, 5); + CheckRowNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( removeRow2 ) +{ + auto table = cpp14::make_unique(); + table->rowAt(0); + BOOST_REQUIRE_NO_THROW(table->removeRow(0)); +} + +BOOST_AUTO_TEST_CASE( removeColumn1 ) +{ + auto table = cpp14::make_unique(); + table->columnAt(5); + BOOST_REQUIRE_NO_THROW(table->removeColumn(2)); + + auto columnCount = table->columnCount(); + + BOOST_REQUIRE_EQUAL(columnCount, 5); + CheckColumnNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( removeColumn2 ) +{ + auto table = cpp14::make_unique(); + table->columnAt(0); + BOOST_REQUIRE_NO_THROW(table->removeColumn(0)); +} + +BOOST_AUTO_TEST_CASE( moveRow1 ) +{ + auto table = cpp14::make_unique(); + table->rowAt(5); + + WTableCell* cell2 = table->elementAt(2, 0); + WTableCell* cell0 = table->elementAt(0, 0); + + BOOST_REQUIRE_NO_THROW(table->moveRow(2, 0)); + + BOOST_REQUIRE_EQUAL(table->elementAt(0, 0), cell2); + BOOST_REQUIRE_EQUAL(table->elementAt(1, 0), cell0); + BOOST_REQUIRE_NE(table->elementAt(2, 0), cell2); + + BOOST_REQUIRE_EQUAL(table->rowCount(), 6); + CheckRowNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( moveRow2, * boost::unit_test::disabled() ) +{ + auto table = cpp14::make_unique(); + table->rowAt(5); + + WTableCell* cell2 = table->elementAt(2, 0); + + BOOST_REQUIRE_NO_THROW(table->moveRow(2, 7)); + + BOOST_REQUIRE_NE(table->elementAt(2, 0), cell2); + BOOST_REQUIRE_EQUAL(table->elementAt(7, 0), cell2); + + BOOST_REQUIRE_EQUAL(table->rowCount(), 8); + CheckRowNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( moveColumn1 ) +{ + auto table = cpp14::make_unique(); + table->columnAt(5); + + WTableCell* cell2 = table->elementAt(0, 2); + WTableCell* cell0 = table->elementAt(0, 0); + + BOOST_REQUIRE_NO_THROW(table->moveColumn(2, 0)); + + BOOST_REQUIRE_EQUAL(table->elementAt(0, 0), cell2); + BOOST_REQUIRE_EQUAL(table->elementAt(0, 1), cell0); + BOOST_REQUIRE_NE(table->elementAt(0, 2), cell2); + + BOOST_REQUIRE_EQUAL(table->columnCount(), 6); + CheckColumnNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( moveColumn2, * boost::unit_test::disabled() ) +{ + auto table = cpp14::make_unique(); + table->columnAt(5); + + WTableCell* cell2 = table->elementAt(0, 2); + + BOOST_REQUIRE_NO_THROW(table->moveColumn(2, 7)); + + BOOST_REQUIRE_EQUAL(table->elementAt(0, 7), cell2); + BOOST_REQUIRE_NE(table->elementAt(0, 2), cell2); + + BOOST_REQUIRE_EQUAL(table->columnCount(), 8); + CheckColumnNumbers(table.get()); +} + +BOOST_AUTO_TEST_CASE( clear ) +{ + auto table = cpp14::make_unique(); + table->elementAt(4, 4); + + BOOST_REQUIRE_NO_THROW(table->clear()); + + BOOST_REQUIRE_EQUAL(table->rowCount(), 0); + BOOST_REQUIRE_EQUAL(table->columnCount(), 0); +} + +BOOST_AUTO_TEST_SUITE_END() From 87540cf068d301359a136410f667e386f0664e18 Mon Sep 17 00:00:00 2001 From: jakub-w Date: Sat, 5 Oct 2019 17:40:05 +0200 Subject: [PATCH 4/4] Improved performance of adding big number of rows to WTable - Added new field to WTableRow - rowNumber_ - to track its position in the table_. - Added WTable::updateRowNumbers() function to update row numbers for rows that need updating. - Modified WTable::insertRow(), WTable::removeRow() and WTable::moveRow() to update row numbers for the rows affected by these operations. - Modified WTableCell::rowNum() to use WTableCell::rowNumber_ if present. Adding rows at the end of the array is now much, much faster. It doesn't need to iterate over every cell anymore. Modified methods from WTable are a little slower than before when removing, inserting or moving rows from the middle of the array. This is because of added overhead of having to update row numbers but this slowdown is negligible. --- src/Wt/WTable.C | 17 +++++++++++++++++ src/Wt/WTable.h | 1 + src/Wt/WTableRow.C | 6 ++++-- src/Wt/WTableRow.h | 1 + 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/Wt/WTable.C b/src/Wt/WTable.C index 4d10062c6b..375a2986ff 100644 --- a/src/Wt/WTable.C +++ b/src/Wt/WTable.C @@ -102,7 +102,11 @@ WTableRow* WTable::insertRow(int row, std::unique_ptr tableRow) for (auto &cell : tableRow->cells_) { widgetAdded(cell.get()); } + + int old_end = rows_.size(); rows_.insert(rows_.begin() + row, std::move(tableRow)); + updateRowNumbers(std::min(old_end, row), rowCount() - 1); + rows_[row].get()->expand(columnCount()); repaint(RepaintFlag::SizeAffected); @@ -143,6 +147,8 @@ std::unique_ptr WTable::removeRow(int row) std::unique_ptr result = std::move(rows_[row]); rows_.erase(rows_.begin() + row); + updateRowNumbers(row, rowCount() - 1); + result->setTable(nullptr); for (auto &cell : result->cells_) @@ -408,6 +414,7 @@ void WTable::moveRow(int from, int to) if (to > (int)rows_.size()) rowAt(to); rows_.insert(rows_.begin() + to, std::move(from_tr)); + updateRowNumbers(std::min(from, to), std::max(from, to)); // make sure spans don't cause segmentation faults during rendering auto& cells = rows_[to]->cells_; @@ -451,6 +458,16 @@ void WTable::moveColumn(int from, int to) repaint(RepaintFlag::SizeAffected); } +void WTable::updateRowNumbers(int begin, int end) { + if (begin < 0 || begin >= rowCount() || end < 0 || end >= rowCount()) { + return; + } + + for (size_t i = begin; i <= end; ++i) { + rows_[i]->rowNumber_ = i; + } +} + void WTable::iterateChildren(const HandleWidgetMethod &method) const { for (const auto &row : rows_) { diff --git a/src/Wt/WTable.h b/src/Wt/WTable.h index bca35f33ea..714d84f977 100644 --- a/src/Wt/WTable.h +++ b/src/Wt/WTable.h @@ -197,6 +197,7 @@ class WT_API WTable : public WInteractWidget void expand(int row, int column, int rowSpan, int columnSpan); WTableCell *itemAt(int row, int column); void setItemAt(int row, int column, std::unique_ptr cell); + void updateRowNumbers(int begin, int end); void repaintRow(WTableRow *row); void repaintColumn(WTableColumn *col); diff --git a/src/Wt/WTableRow.C b/src/Wt/WTableRow.C index d6981618cf..bf0c521cf1 100644 --- a/src/Wt/WTableRow.C +++ b/src/Wt/WTableRow.C @@ -17,8 +17,9 @@ namespace Wt { WTableRow::WTableRow() : hidden_(false), - hiddenChanged_(false) -{ + hiddenChanged_(false), + rowNumber_(-1) +{ implementStateless(&WTableRow::hide, &WTableRow::undoHide); implementStateless(&WTableRow::show, &WTableRow::undoHide); } @@ -87,6 +88,7 @@ WTableCell *WTableRow::elementAt(int column) int WTableRow::rowNum() const { if (table_) + if (rowNumber_ != -1) return rowNumber_; for (unsigned i = 0; i < table_->rows_.size(); ++i) if (table_->rows_[i].get() == this) return i; diff --git a/src/Wt/WTableRow.h b/src/Wt/WTableRow.h index e8884553dc..92a0e2a087 100644 --- a/src/Wt/WTableRow.h +++ b/src/Wt/WTableRow.h @@ -148,6 +148,7 @@ class WT_API WTableRow : public WObject std::string id_; WT_USTRING styleClass_; bool hidden_, hiddenChanged_, wasHidden_; + size_t rowNumber_; void updateDom(DomElement& element, bool all); void setTable(WTable *table);