Skip to content

[Type] Windows: dont export explicit instanciations - #6409

Merged
alxbilger merged 2 commits into
sofa-framework:masterfrom
fredroy:mat_inline_accessors
Oct 29, 2026
Merged

alxbilger merged 2 commits into
sofa-framework:masterfrom
fredroy:mat_inline_accessors

Conversation

@fredroy

@fredroy fredroy commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

... and allow msvc and clang to properly inline hot functions.
In a nutshell, the import/export mechanism (only for win32) block the compiler to properly inline/optimize.
The effect is quite dramatic:

Config fallingSOFA 3instruments_collis TorusFall
MSVC, legacy 82.6 102.5 120.3
clang-cl, legacy 64.7 (×0.78) 112.0 (×1.09) 85.3 (×0.71)
MSVC, inline change 200.7 (×2.43) 119.0 (×1.16) 208.7 (×1.73)
clang-cl, inline change 288.5 (×3.49) 141.3 (×1.38) 204.1 (×1.70)

For TorusFall, clang-ci and optim is slightly slower because the final positions are slightly different (clang-cl use fma by default and compiles with march=native) and causes more constraints (so longer time in constraint resolution, snowball effect)

I edited a little bit the comments the AI had generated but I think it is important so I let them

[with-all-tests]


By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

@fredroy fredroy added pr: enhancement About a possible enhancement pr: status to review To notify reviewers to review this pull-request pr: highlighted in next release Highlight this contribution in the notes of the upcoming release pr: AI-aided Label notifying the reviewers that part or all of the PR has been generated with the help of an AI labels Oct 16, 2026
@fredroy
fredroy force-pushed the mat_inline_accessors branch from 2dd496a to b96dd1d Compare October 16, 2026 04:57
@alxbilger

Copy link
Copy Markdown
Contributor

only Windows?

@fredroy

fredroy commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor Author

only Windows?

Yes, it is because only Windows uses the import/export mechanism

@fredroy
fredroy force-pushed the mat_inline_accessors branch from b96dd1d to ae21515 Compare October 28, 2026 01:51
@hugtalbot hugtalbot added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Oct 29, 2026
…extern templates

On Windows, "extern template class SOFA_TYPE_API Mat<...>" makes every Mat member
dllimport. Neither MSVC nor clang-cl inline those members (clang-cl refuses because
they call non-imported std::array members), so even operator()(i,j) becomes a call
through the import table. clang-cl was additionally hit by
lld placing the exported accessor across a cache line.

Skip the extern template declarations on _WIN32 so client code instantiates the inline
members locally. Mat.cpp keeps its exported explicit instantiations, so Sofa.Type.dll
exports the same symbols and binaries built against the old header still load.
…tialVector, Transform and ClampedScalar

Same reasoning as for Mat: on Windows, the dllimport extern templates keep the
header-defined members from being inlined. Members defined out of line in the .inl
files are still exported by Sofa.Type and are reached through the import library.
Sofa.Type.dll exports are unchanged.
@alxbilger
alxbilger force-pushed the mat_inline_accessors branch from ae21515 to 49eec50 Compare October 29, 2026 13:58
@alxbilger
alxbilger merged commit a5b5c9f into sofa-framework:master Oct 29, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: AI-aided Label notifying the reviewers that part or all of the PR has been generated with the help of an AI pr: enhancement About a possible enhancement pr: highlighted in next release Highlight this contribution in the notes of the upcoming release pr: status ready Approved a pull-request, ready to be squashed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants