You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2023/06/06 02:09:51 UTC
[GitHub] [arrow] kou opened a new pull request, #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
kou opened a new pull request, #35924:
URL: https://github.com/apache/arrow/pull/35924
### Rationale for this change
`-DCMAKE_CXX_FLAGS_RELEASE=-O...` works with #15022 but `-DCMAKE_CXX_FLAGS_DEBUG=-O...` and
`-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=-O...` don't work. Because we always specify `-O0` for Debug and RelWithDebInfo.
### What changes are included in this PR?
These changes don't specify optimization levels for Debug and RelWithDebInfo because optimization flags we specified are the same as the default one. So user specified `CMAKE_CXX_FLAGS_DEBUG=-O...` and `CMAKE_CXX_FLAGS_RELWITHDEBINFO=-O...` are used.
In addition, `ARROW_C_FLAGS_${CONFIG}` and `ARROW_CXX_FLAGS_${CONFIG}` are introduced. Users can also use `-DARROW_CXX_FLAGS_DEBUG=-O...` instead of `-DCMAKE_CXX_FLAGS_DEBUG=-O...`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1577786806
:warning: GitHub issue #35870 **has been automatically assigned in GitHub** to PR creator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1598393061
Thanks for these suggestions!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #35924:
URL: https://github.com/apache/arrow/pull/35924#discussion_r1234965165
##########
docs/source/developers/cpp/building.rst:
##########
@@ -254,6 +254,34 @@ Several build types are possible:
* ``Release``: applies compiler optimizations and removes debug information
from the binary.
+.. note::
+
+ These built types provide suitable optimization/debug flags by
+ default but you can change them by specifying
+ ``-DARROW_C_FLAGS_${BUILD_TYPE}=...`` and/or
+ ``-DARROW_CXX_FLAGS_${BUILD_TYPE}=...``. ``${BUILD_TYPE}`` is upper
+ case of build type. For example, ``DEBUG``
+ (``-DARROW_C_FLAGS_DEBUG=...`` / ``-DARROW_CXX_FLAGS_DEBUG=...``) for the
+ ``Debug`` build type and ``RELWITHDEBINFO``
+ (``-DARROW_C_FLAGS_RELWITHDEBINFO=...`` /
+ ``-DARROW_CXX_FLAGS_RELWITHDEBINFO=...``) for the ``RelWithDebInfo``
+ build type.
+
+ For example, you can use ``-O3`` as an optimization flag by
+ specifying ``-DARROW_CXX_FLAGS_RELEASE=-O3`` for the ``Release``
+ build type. You can use ``-g3`` as a debug flag by specifying
+ ``-DARROW_CXX_FLAGS_DEBUG=-g3`` for the ``Debug`` build type.
Review Comment:
```suggestion
For example, you can use ``-O3`` as an optimization flag for the ``Release``
build type by passing ``-DARROW_CXX_FLAGS_RELEASE=-O3`` .
You can use ``-g3`` as a debug flag for the ``Debug`` build type
by passing ``-DARROW_CXX_FLAGS_DEBUG=-g3`` .
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1593641191
Benchmark runs are scheduled for commit b99fb72593a34256969a3dc14027e00d7b1ccdcf. Watch https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] austin3dickey commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "austin3dickey (via GitHub)" <gi...@apache.org>.
austin3dickey commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1593696635
Runs are also viewable at https://buildkite.com/apache-arrow -- we're making some improvements to the benchmarking notifications!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #35924:
URL: https://github.com/apache/arrow/pull/35924#discussion_r1234958919
##########
docs/source/developers/cpp/building.rst:
##########
@@ -254,6 +254,34 @@ Several build types are possible:
* ``Release``: applies compiler optimizations and removes debug information
from the binary.
+.. note::
+
+ These built types provide suitable optimization/debug flags by
Review Comment:
```suggestion
These build types provide suitable optimization/debug flags by
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1598379199
Good point. I've added it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou merged pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #35924:
URL: https://github.com/apache/arrow/pull/35924
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1593640888
@ursabot please benchmark
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #35924:
URL: https://github.com/apache/arrow/pull/35924#discussion_r1234963665
##########
docs/source/developers/cpp/building.rst:
##########
@@ -254,6 +254,34 @@ Several build types are possible:
* ``Release``: applies compiler optimizations and removes debug information
from the binary.
+.. note::
+
+ These built types provide suitable optimization/debug flags by
+ default but you can change them by specifying
+ ``-DARROW_C_FLAGS_${BUILD_TYPE}=...`` and/or
+ ``-DARROW_CXX_FLAGS_${BUILD_TYPE}=...``. ``${BUILD_TYPE}`` is upper
+ case of build type. For example, ``DEBUG``
+ (``-DARROW_C_FLAGS_DEBUG=...`` / ``-DARROW_CXX_FLAGS_DEBUG=...``) for the
+ ``Debug`` build type and ``RELWITHDEBINFO``
+ (``-DARROW_C_FLAGS_RELWITHDEBINFO=...`` /
+ ``-DARROW_CXX_FLAGS_RELWITHDEBINFO=...``) for the ``RelWithDebInfo``
+ build type.
+
+ For example, you can use ``-O3`` as an optimization flag by
+ specifying ``-DARROW_CXX_FLAGS_RELEASE=-O3`` for the ``Release``
+ build type. You can use ``-g3`` as a debug flag by specifying
+ ``-DARROW_CXX_FLAGS_DEBUG=-g3`` for the ``Debug`` build type.
+
+ You can also use the standard ``-DCMAKE_C_FLAGS_${BUILD_TYPE}=...``
+ and ``-DCMAKE_CXX_FLAGS_${BUILD_TYPE}=...`` style but
Review Comment:
```suggestion
You can also use the standard ``CMAKE_C_FLAGS_${BUILD_TYPE}``
and ``CMAKE_CXX_FLAGS_${BUILD_TYPE}`` variables but
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #35924:
URL: https://github.com/apache/arrow/pull/35924#discussion_r1234963044
##########
docs/source/developers/cpp/building.rst:
##########
@@ -254,6 +254,34 @@ Several build types are possible:
* ``Release``: applies compiler optimizations and removes debug information
from the binary.
+.. note::
+
+ These built types provide suitable optimization/debug flags by
+ default but you can change them by specifying
+ ``-DARROW_C_FLAGS_${BUILD_TYPE}=...`` and/or
+ ``-DARROW_CXX_FLAGS_${BUILD_TYPE}=...``. ``${BUILD_TYPE}`` is upper
+ case of build type. For example, ``DEBUG``
+ (``-DARROW_C_FLAGS_DEBUG=...`` / ``-DARROW_CXX_FLAGS_DEBUG=...``) for the
+ ``Debug`` build type and ``RELWITHDEBINFO``
+ (``-DARROW_C_FLAGS_RELWITHDEBINFO=...`` /
+ ``-DARROW_CXX_FLAGS_RELWITHDEBINFO=...``) for the ``RelWithDebInfo``
+ build type.
+
+ For example, you can use ``-O3`` as an optimization flag by
+ specifying ``-DARROW_CXX_FLAGS_RELEASE=-O3`` for the ``Release``
+ build type. You can use ``-g3`` as a debug flag by specifying
+ ``-DARROW_CXX_FLAGS_DEBUG=-g3`` for the ``Debug`` build type.
+
+ You can also use the standard ``-DCMAKE_C_FLAGS_${BUILD_TYPE}=...``
+ and ``-DCMAKE_CXX_FLAGS_${BUILD_TYPE}=...`` style but
+ ``-DARROW_C_FLAGS_${BUILD_TYPE`` and
+ ``-DARROW_CXX_FLAGS_${BUILD_TYPE}=...`` style is
+ recommended. Because ``-DCMAKE_C_FLAGS_${BUILD_TYPE}=...`` and
+ ``-DCMAKE_CXX_FLAGS_${BUILD_TYPE}=...`` style replaces all default
+ flags provided by CMake. ``-DARROW_C_FLAGS_${BUILD_TYPE`` and
+ ``-DARROW_CXX_FLAGS_${BUILD_TYPE}=...`` style just overrides the
+ default flags specified instead of replacing them.
Review Comment:
```suggestion
the ``ARROW_C_FLAGS_${BUILD_TYPE}`` and
``ARROW_CXX_FLAGS_${BUILD_TYPE}`` variables are
recommended. The ``CMAKE_C_FLAGS_${BUILD_TYPE}`` and
``CMAKE_CXX_FLAGS_${BUILD_TYPE}`` variables replace all default
flags provided by CMake, while ``ARROW_C_FLAGS_${BUILD_TYPE}`` and
``ARROW_CXX_FLAGS_${BUILD_TYPE}`` just append the
flags specified, which allows selectively overriding some of the defaults.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1601054925
Conbench analyzed the 6 benchmark runs on commit `72343f53`.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/14443003671) has more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1592419810
@ursabot please benchmark
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #35924:
URL: https://github.com/apache/arrow/pull/35924#discussion_r1234959312
##########
docs/source/developers/cpp/building.rst:
##########
@@ -254,6 +254,34 @@ Several build types are possible:
* ``Release``: applies compiler optimizations and removes debug information
from the binary.
+.. note::
+
+ These built types provide suitable optimization/debug flags by
+ default but you can change them by specifying
+ ``-DARROW_C_FLAGS_${BUILD_TYPE}=...`` and/or
+ ``-DARROW_CXX_FLAGS_${BUILD_TYPE}=...``. ``${BUILD_TYPE}`` is upper
+ case of build type. For example, ``DEBUG``
+ (``-DARROW_C_FLAGS_DEBUG=...`` / ``-DARROW_CXX_FLAGS_DEBUG=...``) for the
+ ``Debug`` build type and ``RELWITHDEBINFO``
+ (``-DARROW_C_FLAGS_RELWITHDEBINFO=...`` /
+ ``-DARROW_CXX_FLAGS_RELWITHDEBINFO=...``) for the ``RelWithDebInfo``
+ build type.
+
+ For example, you can use ``-O3`` as an optimization flag by
+ specifying ``-DARROW_CXX_FLAGS_RELEASE=-O3`` for the ``Release``
+ build type. You can use ``-g3`` as a debug flag by specifying
+ ``-DARROW_CXX_FLAGS_DEBUG=-g3`` for the ``Debug`` build type.
+
+ You can also use the standard ``-DCMAKE_C_FLAGS_${BUILD_TYPE}=...``
+ and ``-DCMAKE_CXX_FLAGS_${BUILD_TYPE}=...`` style but
Review Comment:
```suggestion
and ``-DCMAKE_CXX_FLAGS_${BUILD_TYPE}=...`` variables, but
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1592419891
Benchmark runs are scheduled for baseline = b4ac585ecb4da610cc64e346e564ca86594aec53 and contender = 8f23df39f89593201ecdbf2333ba413e8688a817. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/c2fc9d2bf0384c33b1a44635072abd9b...c6a98c80792f4866aca3a3d5af1a0677/)
[Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/3ba67613353548eeb97f8547fa50d633...9d6111e771af40948257e1456bf4b5cd/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/61bc471bd1ac4b6ab5fb9a8c8d4d85ee...5542e36d3051419e8612d30652f0389a/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/9786ece2d4bb418c9f5f90a6849d25c3...e5d790c87d0845a1bebe336722129fcb/)
Buildkite builds:
[Scheduled] [`8f23df39` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/3056)
[Scheduled] [`8f23df39` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/3086)
[Scheduled] [`8f23df39` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/3049)
[Scheduled] [`8f23df39` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/3075)
[Finished] [`b4ac585e` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/3053)
[Failed] [`b4ac585e` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/3080)
[Failed] [`b4ac585e` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/3045)
[Failed] [`b4ac585e` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/3070)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #35924: GH-35870: [C++] Add support for changing optimization flags with CMAKE_CXX_FLAGS_DEBUG
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #35924:
URL: https://github.com/apache/arrow/pull/35924#issuecomment-1593880745
Conbench analyzed the 7 benchmark runs on commit `b99fb725`.
There were 6 benchmark results indicating a performance regression:
- Pull Request Run at [2023-06-15 22:03:48Z](http://conbench.ursa.dev/compare/runs/3ba67613353548eeb97f8547fa50d633...410739cfc1634d55b73fd31faddd724d/)
- [params=8192, source=cpp-micro, suite=arrow-bit-util-benchmark](http://conbench.ursa.dev/compare/benchmarks/0648a88086ae76968000b407a5e934cf...0648b8b03b1d73878000d9dc573b86c5)
- Pull Request Run at [2023-06-15 23:09:38Z](http://conbench.ursa.dev/compare/runs/9786ece2d4bb418c9f5f90a6849d25c3...994765593c574b40baa19564d3f5eec3/)
- [params=DecodeArrow_Dense/1024, source=cpp-micro, suite=parquet-encoding-benchmark](http://conbench.ursa.dev/compare/benchmarks/0648a905e2637d2a8000d9623a4af1ff...0648b9ac967c769c8000b3dbe445de65)
- and 4 more (see the report linked below)
The [full Conbench report](https://github.com/apache/arrow/runs/14307811272) has more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org