Add Optimization Flag

[MahirAbbas]

Creates a common_flags library which has compile options. The current flag is now -O2 and -g.

Fixes #542

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.86%. Comparing base (a936eac) to head (03fb193).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #541   +/-   ##
=======================================
  Coverage   47.86%   47.86%           
=======================================
  Files          95       95           
  Lines        7673     7673           
  Branches      669      669           
=======================================
  Hits         3673     3673           
  Misses       3808     3808           
  Partials      192      192           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Ohisemega]

This looks OK to me. Are you OK with adding the flag @ajgdls?
cc: @JamesOHeaDLS

[Ohisemega]

The response from the architecture team is to benchmark this PR in the profiling suite before committing the PR. So the proposed change is good; we would like justification by analysis and numbers.

[MahirAbbas]

Initial data suggests performance improvements are between 6.7% and 18.2%, with Sum plugin showing a 46% decrease in processing time. @Ohisemega

[MahirAbbas]

The current build is now with -O2 and with -g to preserve debug symbols

[Ohisemega]

Based on my comments in PR #544, it might be worth using this method to add the optimisation flag instead:
set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -O2")
The concern is that interface doesn't seem to propagate the flags from the top CMakeLists.txt file!

[Ohisemega]

See last comment about changing from using interface to:
set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -O2")

[MahirAbbas]

@Ohisemega Upon further research and thought, it makes more sense to use the compilation flag inside the Dockerfile. This also leads to the compilation flag being propagated down.

[Ohisemega]

@Ohisemega Upon further research and thought, it makes more sense to use the compilation flag inside the Dockerfile. This also leads to the compilation flag being propagated down.

Hi @MahirAbbas, the changes you made will affect the build container, which is good, but we also need those changes in the developer stage of the Dockerfile. Since the Dockerfile's developer stage doesn't build the binary, we need to modify the CMake files as well.

[MahirAbbas]

@Ohisemega Upon further research and thought, it makes more sense to use the compilation flag inside the Dockerfile. This also leads to the compilation flag being propagated down.

Hi @MahirAbbas, the changes you made will affect the build container, which is good, but we also need those changes in the developer stage of the Dockerfile. Since the Dockerfile's developer stage doesn't build the binary, we need to modify the CMake files as well.

@Ohisemega Is the developer stage used for anything but development? The optimisation will increase compile time (which I was assuming would be quite annoying when developing). It made sense to me to ship an optimised image, and leave the developer experience unoptimised, but I may have been wrong.

[Ohisemega]

@Ohisemega Upon further research and thought, it makes more sense to use the compilation flag inside the Dockerfile. This also leads to the compilation flag being propagated down.

Hi @MahirAbbas, the changes you made will affect the build container, which is good, but we also need those changes in the developer stage of the Dockerfile. Since the Dockerfile's developer stage doesn't build the binary, we need to modify the CMake files as well.

@Ohisemega Is the developer stage used for anything but development? The optimisation will increase compile time (which I was assuming would be quite annoying when developing). It made sense to me to ship an optimised image, and leave the developer experience unoptimised, but I may have been wrong.

It's a fair point you have raised. I'll approve it then!