Skip to content

Topology build speed-ups and (including threading) - #4302

Merged
garth-wells merged 31 commits into
mainfrom
garth/topology-speed-2
Jul 21, 2026
Merged

Topology build speed-ups and (including threading)#4302
garth-wells merged 31 commits into
mainfrom
garth/topology-speed-2

Conversation

@garth-wells

@garth-wells garth-wells commented Jul 19, 2026

Copy link
Copy Markdown
Member

Add supported by threaded mesh graph building and other various performance improvements.

garth-wells and others added 11 commits March 1, 2026 09:50
The experimental replacement for the cell-wise owned-vertex numbering
loop broke correctness: it reset the merge iterator into the globally
sorted owned_vertices array back to cbegin() for every cell, and never
advanced it when a mismatch occurred, so vertices were mismatched to
the wrong position. This caused CHECK_adjacency_list_equal failures in
"Interval adaptive refinement" (interval.cpp) with cyclically-shifted
connectivity.

Restore the previously working, correctness-verified implementation
(the old code was left commented out above the broken block).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@michalhabera

michalhabera commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Make sure the meaning of num_threads is the same in all interfaces. Here for num_threads=1 you do spawn single thread and keep the caller waiting. I do prefer the other approach, where caller does the work and we spawn num_threads-1 threads, as done in #4270. It avoids any possible overhead and does not rely on the OS scheduling to make sure the caller is not assigned resources when it is waiting.

@garth-wells

Copy link
Copy Markdown
Member Author

Make sure the meaning of num_threads is the same in all interfaces. Here for num_threads=1 you do spawn single thread and keep the caller waiting. I do prefer the other approach, where caller does the work and we spawn num_threads-1 threads, as done in #4270. It avoids any possible overhead and does not rely on the OS scheduling to make sure the caller is not assigned resources when it is waiting.

The changes in #4270 are inconsistent with the changes that came before it. Everywhere apart from in #4270, num_threads=0 means spawn no (zero) threads, and num_threads=n (n > 0) means spawn n threads. This also allows non-threaded and threaded code paths to be tested for the same use case of a 'single worker'.

@michalhabera

Copy link
Copy Markdown
Contributor

Make sure the meaning of num_threads is the same in all interfaces. Here for num_threads=1 you do spawn single thread and keep the caller waiting. I do prefer the other approach, where caller does the work and we spawn num_threads-1 threads, as done in #4270. It avoids any possible overhead and does not rely on the OS scheduling to make sure the caller is not assigned resources when it is waiting.

The changes in #4270 are inconsistent with the changes that came before it. Everywhere apart from in #4270, num_threads=0 means spawn no (zero) threads, and num_threads=n (n > 0) means spawn n threads. This also allows non-threaded and threaded code paths to be tested for the same use case of a 'single worker'.

That's not true, see what GJK does,

if (num_threads <= 1)
{
compute_chunk(0, total_size, q);
}

or also what ordering does,
if (nt <= 1)
{
for (std::size_t i = 0; i < S.size(); ++i)
lstmps[i] = create_level_structure(graph, S[i]);
}

Both have for num_threads=1 serial, non-threaded codepath. This has to be unified, thats a primary concern. Other issues we can benchmark and fix uniformly later.

@garth-wells

Copy link
Copy Markdown
Member Author

Update for num_thread consistency.

Comment thread python/dolfinx/mesh.py Outdated
garth-wells and others added 2 commits July 21, 2026 08:52
@garth-wells
garth-wells marked this pull request as ready for review July 21, 2026 06:53
@garth-wells
garth-wells enabled auto-merge July 21, 2026 06:55
@garth-wells
garth-wells added this pull request to the merge queue Jul 21, 2026
Merged via the queue into main with commit 6aab053 Jul 21, 2026
20 checks passed
@garth-wells
garth-wells deleted the garth/topology-speed-2 branch July 21, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants