kindred/create
1
1
Fork 0
Code Issues 37 Pull Requests Actions Packages Projects 9 Releases 2 Wiki Activity

Address the poor performance of the existing unique-name generation (part 1) (#18589)

Browse Source 
* Change Address the poor performance of the existing unique-name generation

As described in Issue 16849, the existing Tools::getUniqueName method
requires calling code to form a vector of existing names to be avoided.

This leads to poor performance both in the O(n) cost of building such a
vector and also getUniqueName's O(n) algorithm for actually generating
the unique name (where 'n' is the number of pre-existing names).

This has  particularly noticeable cost in documents with large numbers
of DocumentObjects because generating both Names and Labels for each new
object incurs this cost. During an operation such as importing this
results in an O(n^2) time spent generating names.

The other major cost is in the saving of the temporary backup file,
which uses name generation for the "files" embedded in the Zip file.
Documents can easily need several such "files" for each object in the
document.

This is the first part of the correction, adding an efficient class for
managing sets of unique names.

New class UniqueNameManager keeps a list of existing names organized in
a manner that eases unique-name generation. This class essentially acts
as a set of names, with the ability to add and remove names and check if
a name is already there, with the added ability to take a prototype name
and generate a unique form for it which is not already in the set.

a new unit test for UniqueNameManager has been added as well.

There is a small regression, compared to the existing unique-name code,
insofar as passing a prototype name like "xyz1234" to the old code would
yield "xyz1235" whether or not "xyz1234" already existed, while the new
code will return the next name above the currently-highest name on the
"xyz" model, which could be "xyz" or "xyz1" or "xyz0042"..
This commit is contained in:
Kevin Martin
2025-01-13 11:57:53 -05:00
committed by GitHub
parent 47ff5a1c7d
commit 346be29042
5 changed files with 537 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

109
tests/src/Base/UniqueNameManager.cpp Normal file
View File

@@ -0,0 +1,109 @@
/***************************************************************************
* Copyright (c) 2024 Kevin Martin <kpmartin@papertrail.ca> *
* *
* This file is part of FreeCAD. *
* *
* FreeCAD is free software: you can redistribute it and/or modify it *
* under the terms of the GNU Lesser General Public License as *
* published by the Free Software Foundation, either version 2.1 of the *
* License, or (at your option) any later version. *
* *
* FreeCAD is distributed in the hope that it will be useful, but *
* WITHOUT ANY WARRANTY; without even the implied warranty of *
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU *
* Lesser General Public License for more details. *
* *
* You should have received a copy of the GNU Lesser General Public *
* License along with FreeCAD. If not, see *
* <https://www.gnu.org/licenses/>. *
* *
***************************************************************************/
#include <gtest/gtest.h>
#include <string>
#include <Base/UniqueNameManager.h>
// NOLINTBEGIN(cppcoreguidelines-*,readability-*)
TEST(UniqueNameManager, NonconflictingBaseNameReturnsSameName)
{
// Check that a model name that is a base name that does not conflict with
// any existing name returns the base name
EXPECT_EQ(Base::UniqueNameManager().makeUniqueName("Body"), "Body");
}
TEST(UniqueNameManager, NonconflictingUniqueNameReturnsBaseName)
{
// Check that a model name that is a unique name whose base name does not conflict with
// any existing name returns the base name
EXPECT_EQ(Base::UniqueNameManager().makeUniqueName("Body123"), "Body");
}
TEST(UniqueNameManager, ManyDigitsInModelNameIgnored)
{
// Check that the number of digits in the model name does not affect the number
// of digits in the returned name.
Base::UniqueNameManager manager;
manager.addExactName("Body");
EXPECT_EQ(manager.makeUniqueName("Body12345", 3), "Body001");
}
TEST(UniqueNameManager, ConflictingBaseNameReturnsUniqueName)
{
// Check that a model name that conflicts with an existing name returns a unique form of
// the base name
Base::UniqueNameManager manager;
manager.addExactName("Body");
EXPECT_EQ(manager.makeUniqueName("Body", 1), "Body1");
}
TEST(UniqueNameManager, ConflictingUniqueNameReturnsNewUniqueName)
{
// Check that a given unique name already in the collection causes the return of
// a new unique name.
Base::UniqueNameManager manager;
manager.addExactName("Body");
manager.addExactName("Body001");
EXPECT_EQ(manager.makeUniqueName("Body001", 3), "Body002");
}
TEST(UniqueNameManager, VerifyUniqueDigitCount)
{
// Check that when a unique name is generated it contains at least the specified number
// of inserted digits
Base::UniqueNameManager manager;
manager.addExactName("Body");
EXPECT_EQ(manager.makeUniqueName("Body", 3), "Body001");
}
TEST(UniqueNameManager, UniqueNameLargerThanAnyName)
{
// Check that the name returned is larger (using digits) that any wxisting name
// even if the given model name is not itself in the collection
Base::UniqueNameManager manager;
manager.addExactName("Body001");
EXPECT_EQ(manager.makeUniqueName("Body", 3), "Body002");
}
TEST(UniqueNameManager, Issue18504)
{
// Check that the management of spans of assigned unique digit values is
// correct
std::string name;
Base::UniqueNameManager manager;
manager.addExactName("Origin007");
manager.addExactName("Origin");
manager.addExactName("Origin008");
manager.addExactName("Origin010");
manager.addExactName("Origin011");
manager.addExactName("Origin013");
manager.addExactName("Origin016");
name = manager.makeUniqueName("Origin", 3);
manager.addExactName(name);
name = manager.makeUniqueName("Origin", 3);
manager.addExactName(name);
name = manager.makeUniqueName("Origin", 3);
EXPECT_NE(name, "Origin010");
}
// NOLINTEND(cppcoreguidelines-*,readability-*)