Try with fvisibility=hidden on clang/GCC #10920
Conversation
it's unreadable otherwise
…g fvisibility=hidden
|
|
|
Well, the silly regression failures were unrelated to us. BeautifulSoup4 had a release 6 hours ago which broke things...
I could reproduce the problem if I installed the latest BS4 version. I pinned back to the previous version and it's happy. I'm verifying on the regression tool codebase and will pin E+ to the latest version of that once I make a release there. I really thought this was a "me" problem after the regression changes last week....what a coincidence! |
|
@Myoldmopar I updated the original post with some timings and a notebook |
|
Thanks @jmarrec, looks like it turned out all clean now. I'll do a quick triple check with develop but expect this to merge in a few minutes. |
Myoldmopar
left a comment
There was a problem hiding this comment.
Yep, all good here! And I tested locally and it's happy too. Merging this, thanks @jmarrec
| - name: Install Regression Tool | ||
| if: always() && matrix.run_regressions && steps.branch_build.outcome != 'failure' # always run this step as long as we actually built | ||
| run: pip install energyplus-regressions>=2.1.0 | ||
| run: pip install energyplus-regressions>=2.1.1 # could rely on the requirements.txt file maybe |
There was a problem hiding this comment.
I added this line to get the updated regression tool, and thus the updated beautifulsoup4 dependency, so that regressions would actually ... work.
|
|
||
| # requirements for the CI regression testing scripts | ||
| energyplus-regressions | ||
| energyplus-regressions>=2.1.1 |
There was a problem hiding this comment.
I'm actually thinking I don't want this in here long term. The regression workflow can install this. No need for it when packaging.
| if not backtrace_shown: | ||
| print("Traceback shown once:") | ||
| print_exc() | ||
| backtrace_shown = True |
There was a problem hiding this comment.
When the regression script has an exception, this will print a traceback which will save me multiple hours of trying to debug and reproduce just to get the call stack of the error. And it only shows it one time to avoid bloating the output even more.
| set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake" ${CMAKE_MODULE_PATH}) | ||
|
|
||
| set(CMAKE_POSITION_INDEPENDENT_CODE ON) | ||
| set(CMAKE_CXX_VISIBILITY_PRESET hidden) |
|
And thanks for the tip, @lefticus !! |
Pull request overview
Original idea is from @lefticus (thanks!)
I built develop, and this branch.
Then I randomly sampled 300 CTests (about 10%, 2905 total).
I ran these ctests 10 times in a row, with -j 1, in order to get more stable timings, so I could take the mean of the 10 runs for each.
Globally, I arrive at a 5% runtime improvement on Ubuntu 24.04, but that may be skewed by some outliers.
Taken another way, if I calculate a runtime improvement % on a test by test basis, then take the mean of that, I arrive at a 3.1% improvement.
Binary size:
nm -C -D Products/libenergyplusapi.so.25.1.0gives 32385 lines in develop, 2435 on this branch.My analysis notebook can be found here: https://gist.github.com/jmarrec/a3388c8898bad65894faebb90c38fba6
Pull Request Author
Reviewer