Fix #5631 - Can't build epmodel on MSVC (LNK1189) - Split openstudiolib.dll into 4 separate DLLs on MSVC#5632
Open
jmarrec wants to merge 4 commits into
Open
Fix #5631 - Can't build epmodel on MSVC (LNK1189) - Split openstudiolib.dll into 4 separate DLLs on MSVC#5632jmarrec wants to merge 4 commits into
jmarrec wants to merge 4 commits into
Conversation
jmarrec
added
severity - Blocker
Developer Issue
Pull Request - Ready for CI
Makes for a smaller binary (and standard compliance is a good thing anyways)
On `os-4.0-dev`, adding `epmodel` alongside `model` pushes the single `openstudiolib.dll` to ~123.8k unique exported symbols. MSVC's import library (and DLL export ordinal table) has a hard 16-bit limit of 65,535 → `LNK1189`.
Removing `model` isn't enough (`epmodel` alone is 60.4k; the pre-epmodel total was already 63.4k).
The only fix is splitting into multiple DLLs — **on Windows/MSVC only**; Linux/macOS keep the single `openstudiolib` unchanged.
Symbol budget per group (from my dumpbin analysis):
| DLL | contents | exported symbols | % of 65,535 |
|---|---|---|---|
| `openstudioutilitieslib` | utilities | ~6.4k | 10% |
| `openstudiomodellib` | model + energyplus | ~53.5k | 82% |
| `openstudioepmodellib` | epmodel | ~60.4k | **92%** |
| `openstudiolib` (keeps name) | airflow, gbxml, sdd, radiance, gltf, epjson, isomodel, osversion, alfalfa, modelica, measure | ~5.5k | 8% |
Rationale for this grouping:
- `epmodel` will eventually replace `model` + `energyplus`; `openstudiomodellib` then gets deleted wholesale — no boundary redesign later.
- `measure` depends on **both** `model` and `epmodel`, so it (and the auxiliaries) must sit above both — it lives in the top-level `openstudiolib`, which keeps its name so **no consumer changes** (CLI, rubyengine, pythonengine, csharp, workflow, all test targets keep linking `openstudiolib`).
- Dependency DAG verified clean (no cycles): `utilities ← {model+energyplus, epmodel} ← auxiliaries/measure`. energyplus is used by epjson/gbxml/sdd (all in the top DLL). epmodel is used only by measure/workflow/cli. Link order is not an issue for DLLs.
- ⚠️ `openstudioepmodellib` is born at 92% of the limit and epmodel grows with each E+ release. Not solved here; future mitigation = trim exports (e.g. detail impls) or split epmodel internally.
## Design
**Components stay OBJECT libraries on all platforms** — no changes to any `src/<component>/CMakeLists.txt` library type. The split is done purely at the composition layer in `src/lib/CMakeLists.txt`: on MSVC, create 4 `SHARED` libs each folding its OBJECT libs `PRIVATE` (same `empty.cpp` pattern as today); on other platforms, keep the current single target.
The existing per-component export macros (`UTILITIES_API`, `MODEL_API`, `EPMODEL_API`, … defined via `openstudio_<name>_EXPORTS` set PRIVATE per OBJECT lib) already produce correct dllexport/dllimport across DLL boundaries, since each OBJECT lib lands in exactly one DLL.
…ly dependent with openstudio_model (Model.cpp calls VersionTranslator, and osversion links model_depends).
Several targets (measure/epmodel/workflow test executables, python and ruby engine tests, the epmodel python SWIG modules) linked both openstudio_epmodel's raw object files directly and openstudiolib, which now PUBLIC-links openstudioepmodellib on MSVC. That embeds epmodel's symbols twice: once as real definitions, once as the DLL's import-lib thunks, which MSVC rejects. Add an INTERFACE-only openstudio_epmodel_headers target that carries epmodel's per-subfolder include directories without its object files, and use it wherever a consumer only needs to compile against epmodel headers rather than embed its objects.
jmarrec
force-pushed
the
5631-os-dev-4.0-split_DLL
branch
from
July 13, 2026 22:06
25cda2a to
27847d6
Compare
Contributor
🧪 Test Results DashboardSummary
|
| Run | XML File | Status |
|---|---|---|
| run1 | results.xml |
✅ Found |
| run3 | results.xml |
✅ Found |
| run2 | results.xml |
✅ Found |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request overview
Pull Request Author
src/model/test)src/energyplus/Test)src/osversion/VersionTranslator.cpp)Labels:
IDDChangeAPIChangePull Request - Ready for CIso that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.