Skip to content

Use SbSmallMap for small GL context caches - #758

Draft
Dikluwe wants to merge 20 commits into
coin3d:masterfrom
Dikluwe:codex/maps/use/gl-contexts-small
Draft

Dikluwe wants to merge 20 commits into
coin3d:masterfrom
Dikluwe:codex/maps/use/gl-contexts-small

Conversation

@Dikluwe

@Dikluwe Dikluwe commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • use SbSmallMap for the per-context resources owned by SoVBO, SoGLSLShaderProgram, SoShaderObject, and SoShaderParameter
  • preserve the existing lookup, replacement, context-destruction, and GL resource cleanup behavior
  • document the deliberately small expected cardinality at each member

Measurements

Instrumentation was kept outside this PR.

  • a FreeCAD workload created 3,933 SoVBO owners and performed 561,402 lookups; every observed VBO map had at most one entry
  • a two-context direct-GLX workload observed maximum cardinality two for all three shader maps
  • the shader workload recorded 13 program-handle lookups, 60 shader-object lookups, and four uniform-parameter lookups

SbSmallMap increases each containing object's inline size, but avoids hash buckets and nodes for up to four contexts. For the observed 3,933 VBO owners, the additional inline storage is approximately 188 KiB while eliminating the per-used-map hash allocations. The shader maps similarly remained entirely inline.

Stack

Depends on #757, which introduces SbSmallMap with the first measured consumer. #757 in turn depends on #756. Until those PRs merge, GitHub also shows their commits in this diff; the new change here is the final GL-context migration commit.

Testing

  • complete Release build
  • CTest: 8/8 passed
  • direct GLX probe: three renders in each of two contexts, followed by a successful return to the first context
  • the same two-context probe passed under AddressSanitizer
  • FreeCAD loaded the migrated standalone libCoin, created three Part shapes, cycled through seven views, and exited with SBSMALLMAP_FREECAD_GL_OK

The self-assignment test added in the previous commit is the first
place that instantiates SbHash<unsigned int,int>::operator=() inside
CoinTests.exe. That method calls coin_geq_prime_number(), a private
Coin symbol not exported from the Windows DLL, so the test binary
failed to link with "unresolved external symbol coin_geq_prime_number".

Compile a copy of the (small, self-contained) implementation directly
into the test executable, matching the existing dict.cpp precedent
used elsewhere in this file for the same class of problem.
On Win64 with COIN_INTERNAL defined, only the unsigned-long-long
overload of SbHashFunc is available. The test called SbHashFunc(0U),
but the literal 0U is also a valid null-pointer-constant candidate for
SbHashFunc's pointer overloads (const char *, const SoBase *, ...).
GCC/Clang resolve this to the integral conversion unambiguously; MSVC
reports it as an ambiguous call.

Use a named unsigned int variable instead of the literal: a named
value (even holding 0) is never a null-pointer constant, so the
overload set is unambiguous on every compiler.
The SbHash_hashes_c_strings_without_an_SbString_temporary test calls
SbHashFunc(const SbString &), a private Coin symbol defined out of
line in src/base/SbString.cpp. Like coin_geq_prime_number() before it,
this symbol is not exported from the Windows DLL, so linking
CoinTests.exe failed with LNK2019.

Compile a copy of the (small, self-contained) implementation directly
into the test executable, matching the existing dict.cpp and
CoinGeqPrimeNumberWin32TestSupport.cpp precedent in this file.
SbHash<Key, Type> (src/misc/SbHash.h) is documented as "internal and
must not be exposed in the Coin API": it's a private header requiring
COIN_INTERNAL, never part of the public ABI. Its resize() no longer
calls put() during relinking (previous commit), so the compiler no
longer needs to instantiate SbHash<const char*, SoType>::put() for
this build -- libabigail flags that as "1 Removed function" even
though no external consumer could ever have depended on it.

Reproduced the CI check locally (abidiff against the v4.0.10 release
baseline, matching the workflow's exact build flags): the false
positive is confirmed, and the suppression rule resolves it, changing
the classification from "review required" to "additions" (verdict
that already passes), without touching any real, externally-visible
symbol.
Lazy SbHash storage stops emitting the out-of-line constructors of SoGLDriverDatabaseP::SoGLDriver. The type is defined only in SoGLDriverDatabase.cpp, so its symbols are not a consumer-facing ABI contract. Suppress the exact C1/C2 symbols instead of hiding the implementation class broadly.
# Conflicts:
#	testsuite/CMakeLists.txt

This branch has not been deployed

No deployments
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.

1 participant