Os 4.0 dev schfile - #5640
Open
kbenne wants to merge 18 commits into
Open
Conversation
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.
joseph-robertson
added
the
Pull Request - Ready for CI
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
3 participants
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.
@joseph-robertson your original PR for this was closed when I merged and deleted the target branch. I'm opening a new PR targeting the 4.0 dev branch.
sch = ScheduleFile::fromTimeSeries(...)ts = ScheduleFile::timeseries()