russell@unturf.com edf083b2c4 openfoam+stella: 5-MOAD scan; openfoam-0002 CWE-407, stella CLEAN

openfoam-0002: CFCFaceToCellStencil::calcCellStencil allGlobalFaces dedup
uses findIndex() O(G) linear scan inside nested forAll(cells)*forAll(faces)
loop. Developer left comment "Note:should use hashset?" — confirmed defect.
Fix: labelHashSet seen replaces findIndex, 77x op-count speedup at C=2000/F=12.

stella: all 5 MOADs CLEAN. BreakpointMap uses unordered_map O(1); TrapArray
uses std::array<uInt8,0x10000> O(1); single-threaded, no credentials, no
thread-local identity, no thundering herd.

2026-03-31 21:55:06 -04:00

3 KiB

Raw Blame History

openfoam-0002: CFCFaceToCellStencil calcCellStencil allGlobalFaces dedup O(CFG)

Target

OpenFOAM-dev (https://github.com/OpenFOAM/OpenFOAM-dev)

File

src/finiteVolume/fvMesh/extendedStencil/faceToCell/globalIndexStencils/CFCFaceToCellStencil.C

Function

Foam::CFCFaceToCellStencil::calcCellStencil(labelListList&)

MOAD

0001 — CWE-407 Algorithmic Complexity

Severity

HIGH

Pattern

DynamicList<label> allGlobalFaces(100);

forAll(globalCellFaces, celli)          // O(C) — all mesh cells
{
    allGlobalFaces.clear();
    // ... add own faces ...

    forAll(cFaces, i)                   // O(F) — faces per cell (~6-12 hex)
    {
        const cell& nbrFaces = ...;
        forAll(nbrFaces, j)             // O(F) — neighbour faces
        {
            label nbrGlobalI = ...;

            // Check if already there. Note:should use hashset?
            if (findIndex(allGlobalFaces, nbrGlobalI) == -1)  // O(G)
            {
                allGlobalFaces.append(nbrGlobalI);
            }
        }
    }
}

findIndex is a linear scan over allGlobalFaces (a DynamicList<label>). Per cell, G grows to O(F + F*F_nbr) = O(F²) face entries. The total complexity is O(C * F² * G) = O(C * F³) vs O(C * F²) with a hash set.

Even the OpenFOAM developers noticed: comment reads "Note:should use hashset?" — confirmed defect, never fixed.

Complexity

Version	Complexity	At C=1M cells, F=12
Defective	O(C * F * G), G=O(F²)	~1.7 billion findIndex calls
Fixed	O(C * F) with O(1) set ops	~144 million ops
Speedup	~12x	(theoretical at F=12)

Fix

Replace DynamicList<label> allGlobalFaces dedup-via-findIndex with a labelHashSet seen for O(1) membership. allGlobalFaces is kept for ordered output.

DynamicList<label> allGlobalFaces(100);
labelHashSet seen(100);

forAll(globalCellFaces, celli)
{
    allGlobalFaces.clear();
    seen.clear();
    // ... use seen.insert(globalI) instead of findIndex check ...
}

Impact

CFCFaceToCellStencil is used to build stencils for extended interpolation schemes (LUST, upwind-biased schemes, limiters) on unstructured meshes. It runs once per mesh load or topology change. For large industrial CFD cases with millions of cells this startup phase can take tens of seconds. Patch eliminates O(F²) factor per cell.

MOADs 0002-0005

0002 Intertangle: objectRegistry is a god object but uses HashTable — CLEAN.
0003 Leaked Context: No thread-local request identity. OpenFOAM uses MPI, not threads. OFstreamCollator spawns one background write thread with std::mutex — CLEAN.
0004 CWE-312: No credentials in log streams found. Zoltan parameter logging is config key/value only, not secrets — CLEAN.
0005 Thundering Herd: Lazy singletons (unitsDictPtr_, dimensionedConstantsDictPtr_) have no mutex but OpenFOAM is single-threaded (MPI-parallel) — no thundering herd risk — CLEAN.

3 KiB Raw Blame History