Skip to content

Commit 9626a26

Browse files
committed
Revert "No need for mutable, Remove flaky tests"
This reverts commit 4b13837.
1 parent eb5f6c0 commit 9626a26

File tree

3 files changed

+128
-2
lines changed

3 files changed

+128
-2
lines changed

libyul/optimiser/VarNameCleaner.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@ YulName VarNameCleaner::findCleanName(YulName const& _name) const
104104

105105
// Use a per-base-name counter to avoid O(n²) probing when many
106106
// variables share the same stripped base name.
107-
size_t nextSuffix = util::valueOrDefault(m_nextSuffix, newName, static_cast<size_t>(1));
107+
size_t& nextSuffix = m_nextSuffix.at(newName);
108+
if (nextSuffix == 0)
109+
nextSuffix = 1;
108110
for (; nextSuffix < std::numeric_limits<size_t>::max(); ++nextSuffix)
109111
{
110112
YulName newNameSuffixed = YulName{newName.str() + "_" + std::to_string(nextSuffix)};

libyul/optimiser/VarNameCleaner.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ class VarNameCleaner: public ASTModifier
9090
std::set<YulName> m_usedNames;
9191

9292
/// Next suffix to try per stripped base name, avoids O(n²) probing.
93-
std::map<YulName, size_t> m_nextSuffix;
93+
mutable std::map<YulName, size_t> m_nextSuffix;
9494

9595
/// Maps old to new names.
9696
std::map<YulName, YulName> m_translatedNames;

test/libyul/VarNameCleanerPerf.cpp

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/*
2+
This file is part of solidity.
3+
4+
solidity is free software: you can redistribute it and/or modify
5+
it under the terms of the GNU General Public License as published by
6+
the Free Software Foundation, either version 3 of the License, or
7+
(at your option) any later version.
8+
9+
solidity is distributed in the hope that it will be useful,
10+
but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
GNU General Public License for more details.
13+
14+
You should have received a copy of the GNU General Public License
15+
along with solidity. If not, see <http://www.gnu.org/licenses/>.
16+
*/
17+
// SPDX-License-Identifier: GPL-3.0
18+
/**
19+
* Performance tests for VarNameCleaner to verify O(n) behaviour
20+
* when many variables share the same stripped base name.
21+
*/
22+
23+
#include <test/Common.h>
24+
#include <test/libyul/Common.h>
25+
26+
#include <libyul/optimiser/VarNameCleaner.h>
27+
#include <libyul/optimiser/Disambiguator.h>
28+
#include <libyul/optimiser/FunctionGrouper.h>
29+
#include <libyul/optimiser/FunctionHoister.h>
30+
#include <libyul/optimiser/NameDispenser.h>
31+
#include <libyul/AST.h>
32+
#include <libyul/Object.h>
33+
#include <libyul/YulStack.h>
34+
#include <libyul/Dialect.h>
35+
#include <libyul/backends/evm/EVMDialect.h>
36+
#include <libyul/optimiser/OptimiserStep.h>
37+
38+
#include <boost/test/unit_test.hpp>
39+
40+
#include <chrono>
41+
#include <sstream>
42+
#include <string>
43+
44+
using namespace solidity::langutil;
45+
46+
namespace solidity::yul::test
47+
{
48+
49+
namespace
50+
{
51+
52+
/// Build a Yul block with @a n variables all sharing the same base name "x":
53+
/// { let x_100 := 1 let x_200 := 2 ... let x_(n*100) := n }
54+
/// Using large suffix gaps so stripSuffix strips them all to "x".
55+
std::string buildManyVarsYul(size_t n)
56+
{
57+
std::ostringstream src;
58+
src << "{\n";
59+
for (size_t i = 1; i <= n; ++i)
60+
src << " let x_" << (i * 100) << " := " << i << "\n";
61+
src << "}\n";
62+
return src.str();
63+
}
64+
65+
/// Run VarNameCleaner on the given Yul source and return the elapsed time.
66+
std::chrono::microseconds runVarNameCleaner(std::string const& _source)
67+
{
68+
auto block = disambiguate(_source);
69+
Dialect const& d = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{}, std::nullopt);
70+
std::set<YulName> reserved;
71+
NameDispenser dispenser(d, block, reserved);
72+
OptimiserStepContext context{d, dispenser, reserved, 0};
73+
FunctionHoister::run(context, block);
74+
FunctionGrouper::run(context, block);
75+
76+
auto start = std::chrono::high_resolution_clock::now();
77+
VarNameCleaner::run(context, block);
78+
auto end = std::chrono::high_resolution_clock::now();
79+
80+
return std::chrono::duration_cast<std::chrono::microseconds>(end - start);
81+
}
82+
83+
}
84+
85+
BOOST_AUTO_TEST_SUITE(YulVarNameCleanerPerf)
86+
87+
// Verify that VarNameCleaner scales linearly, not quadratically.
88+
// We compare the runtime of N=2000 vs N=500. With the old O(n^2) code
89+
// the ratio would be ~16x; with the fix it should be close to 4x (linear).
90+
// We use a generous 8x threshold to avoid flaky failures.
91+
BOOST_AUTO_TEST_CASE(linear_scaling)
92+
{
93+
size_t const smallN = 500;
94+
size_t const largeN = 2000;
95+
96+
std::string smallSrc = buildManyVarsYul(smallN);
97+
std::string largeSrc = buildManyVarsYul(largeN);
98+
99+
// Warm up
100+
runVarNameCleaner(smallSrc);
101+
102+
auto smallTime = runVarNameCleaner(smallSrc);
103+
auto largeTime = runVarNameCleaner(largeSrc);
104+
105+
// With O(n) scaling and n ratio of 4x, we expect time ratio ~4x.
106+
// With O(n^2), the ratio would be ~16x.
107+
// Allow up to 8x to avoid flaky test failures while still catching quadratic.
108+
double ratio = static_cast<double>(largeTime.count()) /
109+
static_cast<double>(std::max(smallTime.count(), decltype(smallTime.count()){1}));
110+
111+
BOOST_TEST_MESSAGE("VarNameCleaner: N=" << smallN << " took " << smallTime.count() << " us");
112+
BOOST_TEST_MESSAGE("VarNameCleaner: N=" << largeN << " took " << largeTime.count() << " us");
113+
BOOST_TEST_MESSAGE("Ratio: " << ratio << "x (expected ~4x for linear, ~16x for quadratic)");
114+
115+
BOOST_CHECK_MESSAGE(ratio < 8.0,
116+
"VarNameCleaner appears to scale worse than linearly: "
117+
"ratio=" << ratio << "x for " << largeN << "/" << smallN << " variables. "
118+
"Expected <8x for O(n), got " << ratio << "x."
119+
);
120+
}
121+
122+
BOOST_AUTO_TEST_SUITE_END()
123+
124+
}

0 commit comments

Comments
 (0)