Skip to content

Size two init containers for what they will actually receive - #2833

Merged
grospelliergilles merged 2 commits into
arcaneframework:mainfrom
thdionisi:dev/td-opt-init-size-containers
Oct 21, 2026
Merged

grospelliergilles merged 2 commits into
arcaneframework:mainfrom
thdionisi:dev/td-opt-init-size-containers

Conversation

@thdionisi

@thdionisi thdionisi commented Oct 19, 2026 •

Copy link
Copy Markdown

Two independent one-file fixes on the same theme. ItemInternalMap::resize() was documented as resizing the hash table but had an empty body, so the five callers that announce a batch size were not served. And the size given to a HashTableMapT is a number of buckets, not of entries, so the generator's node table always outgrew itself.
Mesh build −5.3 %, initialisation −9.4 %, on a 640k-cell cartesian mesh at one rank.

@codecov

codecov Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.41%. Comparing base (ab0f8bc) to head (a7663fd).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
arcane/src/arcane/mesh/ItemInternalMap.h 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2833      +/-   ##
==========================================
- Coverage   71.42%   71.41%   -0.02%     
==========================================
  Files        2577     2562      -15     
  Lines      195037   194525     -512     
  Branches    23135    23051      -84     
==========================================
- Hits       139312   138925     -387     
+ Misses      47068    46968     -100     
+ Partials     8657     8632      -25     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@grospelliergilles grospelliergilles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.
Just use C++ static_cast instead of the C style cast

void resize(Int32 new_size, [[maybe_unused]] bool use_prime = false)
{
if (new_size > 0)
m_new_impl.reserve((uint64_t)new_size, false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use static_cast instead of C cast

// The size of a HashTableMapT is a number of buckets and not a number of
// entries: it rehashes as soon as its count passes 0.85 * nb_bucket, so the
// table is sized for the load factor and holds every node without rehashing.
const Int64 wanted_bucket = (Int64)(own_nb_node_xyz / 0.85) + 1;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use static_cast instead of C cast

@thdionisi
thdionisi force-pushed the dev/td-opt-init-size-containers branch from 86c2157 to 6c343c7 Compare October 20, 2026 12:23
Thomas Dionisi added 2 commits October 20, 2026 18:35
Documented as "Resizes the hash table", its body was empty, so the five callers
that announce a batch size were not served and the table grew by doubling.

5.3% gain on the mesh build, 8.8% on addCells itself, on a 640k-cell cartesian
mesh at one rank.
The size given to a HashTableMapT is a number of buckets, and it rehashes past
0.85 * nb_bucket, so a table sized to the number of entries it will receive
always outgrows itself. The mesh is unchanged.

9.4% gain on the initialisation of a 640k-cell cartesian mesh at one rank.
@grospelliergilles
grospelliergilles force-pushed the dev/td-opt-init-size-containers branch from 6c343c7 to a7663fd Compare October 20, 2026 16:35
@grospelliergilles
grospelliergilles self-requested a review October 20, 2026 16:35
@sonarqubecloud

Copy link
Copy Markdown

@grospelliergilles
grospelliergilles merged commit 88cf53e into arcaneframework:main Oct 21, 2026
69 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants