Skip to content

Commit fe45caf

Browse files
Copilotjll63
andcommitted
Address code review feedback: add named constants and fix trace checking
Co-authored-by: jll63 <[email protected]>
1 parent 88984a1 commit fe45caf

File tree

1 file changed

+21
-12
lines changed

1 file changed

+21
-12
lines changed

include/boost/openmethod/policies/minimal_perfect_hash.hpp

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,8 @@ void minimal_perfect_hash::fn<Registry>::initialize(
189189

190190
const auto N = std::distance(ctx.classes_begin(), ctx.classes_end());
191191

192-
if constexpr (mp11::mp_contains<mp11::mp_list<Options...>, trace>::value) {
193-
Registry::output::os << "Finding minimal perfect hash using PtHash for " << N << " types\n";
192+
if constexpr (InitializeContext::template has_option<trace>) {
193+
ctx.tr << "Finding minimal perfect hash using PtHash for " << N << " types\n";
194194
}
195195

196196
// Table size is exactly N for minimal perfect hash
@@ -208,11 +208,12 @@ void minimal_perfect_hash::fn<Registry>::initialize(
208208

209209
if (table_size == 1) {
210210
// Special case: only one type
211-
shift = 8 * sizeof(type_id);
211+
constexpr std::size_t bits_per_type_id = 8 * sizeof(type_id);
212+
shift = bits_per_type_id;
212213
mult = 1;
213214
num_groups = 1;
214215
group_mult = 1;
215-
group_shift = 8 * sizeof(type_id);
216+
group_shift = bits_per_type_id;
216217
detail::minimal_perfect_hash_displacements<Registry>.assign(1, 0);
217218
buckets.resize(1);
218219
for (auto iter = ctx.classes_begin(); iter != ctx.classes_end(); ++iter) {
@@ -233,13 +234,21 @@ void minimal_perfect_hash::fn<Registry>::initialize(
233234
}
234235
}
235236

236-
std::default_random_engine rnd(13081963);
237+
// Constants for PtHash algorithm
238+
constexpr std::size_t DEFAULT_RANDOM_SEED = 13081963; // Same seed as fast_perfect_hash
239+
constexpr std::size_t MAX_PASSES = 10;
240+
constexpr std::size_t MAX_ATTEMPTS = 100000;
241+
constexpr std::size_t DEFAULT_GROUP_DIVISOR = 4; // N/4 groups for balance between memory and speed
242+
constexpr std::size_t DISTRIBUTION_FACTOR = 2; // 2*N range for better distribution
243+
constexpr std::size_t bits_per_type_id = 8 * sizeof(type_id);
244+
245+
std::default_random_engine rnd(DEFAULT_RANDOM_SEED);
237246
std::uniform_int_distribution<std::size_t> uniform_dist;
238247
std::size_t total_attempts = 0;
239248

240249
// PtHash algorithm: partition keys into groups, then find displacements
241250
// Number of groups: typically sqrt(N) to N/4 for good performance
242-
num_groups = (std::max)(std::size_t(1), table_size / 4);
251+
num_groups = (std::max)(std::size_t(1), table_size / DEFAULT_GROUP_DIVISOR);
243252
if (num_groups > table_size) num_groups = table_size;
244253

245254
// Calculate bits needed for num_groups
@@ -249,25 +258,25 @@ void minimal_perfect_hash::fn<Registry>::initialize(
249258
power <<= 1;
250259
++GM;
251260
}
252-
group_shift = 8 * sizeof(type_id) - GM;
261+
group_shift = bits_per_type_id - GM;
253262

254263
if constexpr (InitializeContext::template has_option<trace>) {
255264
ctx.tr << " Using " << num_groups << " groups for " << table_size << " keys\n";
256265
}
257266

258267
// Try different pilot hash parameters
259-
for (std::size_t pass = 0; pass < 10 && total_attempts < 100000; ++pass) {
268+
for (std::size_t pass = 0; pass < MAX_PASSES && total_attempts < MAX_ATTEMPTS; ++pass) {
260269
mult = uniform_dist(rnd) | 1;
261270
group_mult = uniform_dist(rnd) | 1;
262271

263272
// Calculate M for pilot hash (number of bits for table_size range)
264273
std::size_t M = 0;
265274
power = 1;
266-
while (power < table_size * 2) { // Use 2*N for better distribution
275+
while (power < table_size * DISTRIBUTION_FACTOR) {
267276
power <<= 1;
268277
++M;
269278
}
270-
shift = 8 * sizeof(type_id) - M;
279+
shift = bits_per_type_id - M;
271280

272281
// Partition keys into groups
273282
std::vector<std::vector<type_id>> groups(num_groups);
@@ -295,9 +304,9 @@ void minimal_perfect_hash::fn<Registry>::initialize(
295304

296305
// Try different displacement values
297306
bool found = false;
298-
for (std::size_t disp = 0; disp < table_size * 2 && !found; ++disp) {
307+
for (std::size_t disp = 0; disp < table_size * DISTRIBUTION_FACTOR && !found; ++disp) {
299308
++total_attempts;
300-
if (total_attempts > 100000) {
309+
if (total_attempts > MAX_ATTEMPTS) {
301310
success = false;
302311
break;
303312
}

0 commit comments

Comments
 (0)