You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2022/12/11 20:50:58 UTC
[GitHub] [celix] pnoltes opened a new pull request, #454: Feature/update debug optimalization flag
pnoltes opened a new pull request, #454:
URL: https://github.com/apache/celix/pull/454
Add -Og flag when Debug type is used, with exception of libdfi.
libdfi and the test code for libdfi can result in segfault when using optimization
--
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: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a diff in pull request #454: Feature/update debug optimalization flag
Posted by GitBox <gi...@apache.org>.
pnoltes commented on code in PR #454:
URL: https://github.com/apache/celix/pull/454#discussion_r1045673712
##########
libs/dfi/CMakeLists.txt:
##########
@@ -39,7 +39,7 @@ target_include_directories(dfi PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>
$<INSTALL_INTERFACE:include/celix/dfi>
)
-
+target_compile_options(dfi PRIVATE -O0) #Remove optimalization to ensure libdfi / ffi works
Review Comment:
Correct, I think any optimization when using libdfi can be an issue.
I also noticed that the Ubuntu nightly builds are failing on the dfi tests. The nightly builds are configured for a RelWithDebInfo build type.
I agree that this should be an separate issue. I will retest if this the "-O0" can be removed the libdfi, because I expect it is only needed for test code.
I also expect that the optimization gives issues when combined with using ffi for dynamic proxy functions (dynamically creating a stdcall stack). So I will also try to isolate the problematic tests and create a libdfi ticket with this info.
--
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: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] PengZheng commented on a diff in pull request #454: Feature/update debug optimalization flag
Posted by GitBox <gi...@apache.org>.
PengZheng commented on code in PR #454:
URL: https://github.com/apache/celix/pull/454#discussion_r1045507007
##########
libs/dfi/CMakeLists.txt:
##########
@@ -39,7 +39,7 @@ target_include_directories(dfi PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>
$<INSTALL_INTERFACE:include/celix/dfi>
)
-
+target_compile_options(dfi PRIVATE -O0) #Remove optimalization to ensure libdfi / ffi works
Review Comment:
Does it has anything to do with `-Og`? I guess not.
Does it crash when using `-O2`? If so, we should investigate why it happens.
More background will be helpful. Maybe we file an issue regarding this?
--
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: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on pull request #454: Feature/update debug optimalization flag
Posted by GitBox <gi...@apache.org>.
pnoltes commented on PR #454:
URL: https://github.com/apache/celix/pull/454#issuecomment-1345653882
From gcc documentation
> -Og
> Optimize debugging experience. -Og should be the optimization level of choice for the standard edit-compile-debug cycle, offering a reasonable level of optimization while maintaining fast compilation and a good debugging experience. It is a better choice than -O0 for producing debuggable code because some compiler passes that collect debug information are disabled at -O0.
--
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: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-commenter commented on pull request #454: Feature/update debug optimalization flag
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #454:
URL: https://github.com/apache/celix/pull/454#issuecomment-1345655324
# [Codecov](https://codecov.io/gh/apache/celix/pull/454?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#454](https://codecov.io/gh/apache/celix/pull/454?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce9aacc) into [master](https://codecov.io/gh/apache/celix/commit/646e32eef2b1581175e0f117690150da37933c26?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (646e32e) will **increase** coverage by `1.06%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #454 +/- ##
==========================================
+ Coverage 74.49% 75.55% +1.06%
==========================================
Files 199 202 +3
Lines 29616 29505 -111
==========================================
+ Hits 22061 22292 +231
+ Misses 7555 7213 -342
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/454?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [bundles/shell/shell/api/celix/IShellCommand.h](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9hcGkvY2VsaXgvSVNoZWxsQ29tbWFuZC5o) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [libs/promises/api/celix/IScheduledExecutor.h](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy9wcm9taXNlcy9hcGkvY2VsaXgvSVNjaGVkdWxlZEV4ZWN1dG9yLmg=) | `87.50% <0.00%> (-12.50%)` | :arrow_down: |
| [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `92.30% <0.00%> (-7.70%)` | :arrow_down: |
| [libs/framework/src/celix\_errorcodes.c](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2Vycm9yY29kZXMuYw==) | `30.00% <0.00%> (-6.37%)` | :arrow_down: |
| [libs/utils/src/celix\_log\_utils.c](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy91dGlscy9zcmMvY2VsaXhfbG9nX3V0aWxzLmM=) | `83.75% <0.00%> (-1.97%)` | :arrow_down: |
| [libs/framework/include/celix/BundleContext.h](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9CdW5kbGVDb250ZXh0Lmg=) | `98.27% <0.00%> (-1.73%)` | :arrow_down: |
| [libs/framework/src/service\_reference.c](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVmZXJlbmNlLmM=) | `55.49% <0.00%> (-1.49%)` | :arrow_down: |
| [bundles/http\_admin/http\_admin/src/activator.c](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2FjdGl2YXRvci5j) | `91.22% <0.00%> (-1.23%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_utils.c](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfdXRpbHMuYw==) | `68.81% <0.00%> (-1.19%)` | :arrow_down: |
| [libs/utils/include/celix/Properties.h](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGlicy91dGlscy9pbmNsdWRlL2NlbGl4L1Byb3BlcnRpZXMuaA==) | `94.64% <0.00%> (-0.94%)` | :arrow_down: |
| ... and [133 more](https://codecov.io/gh/apache/celix/pull/454/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on pull request #454: Feature/update debug optimalization flag
Posted by GitBox <gi...@apache.org>.
pnoltes commented on PR #454:
URL: https://github.com/apache/celix/pull/454#issuecomment-1380117537
> @xuzhenbao Please have a look at libdfi.
I would like to merge this PR, because IMO the libdfi issue can be investigated and fix on a separate branch.
IMO this PR does not introduce libdfi problems, but only brings it to the surface.
--
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: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] PengZheng merged pull request #454: Feature/update debug optimalization flag
Posted by GitBox <gi...@apache.org>.
PengZheng merged PR #454:
URL: https://github.com/apache/celix/pull/454
--
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: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a diff in pull request #454: Feature/update debug optimalization flag
Posted by GitBox <gi...@apache.org>.
pnoltes commented on code in PR #454:
URL: https://github.com/apache/celix/pull/454#discussion_r1045673712
##########
libs/dfi/CMakeLists.txt:
##########
@@ -39,7 +39,7 @@ target_include_directories(dfi PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/include>
$<INSTALL_INTERFACE:include/celix/dfi>
)
-
+target_compile_options(dfi PRIVATE -O0) #Remove optimalization to ensure libdfi / ffi works
Review Comment:
Correct, I think any optimization when using libdfi can be an issue.
I also noticed that the Ubuntu nightly builds are failing on the dfi tests. The nightly builds are configured for a RelWithDebInfo build type. (https://github.com/apache/celix/actions/runs/3671441824)
I agree that this should be an separate issue. I will retest if this the "-O0" can be removed the libdfi, because I expect it is only needed for test code.
I also expect that the optimization gives issues when combined with using ffi for dynamic proxy functions (dynamically creating a stdcall stack). So I will also try to isolate the problematic tests and create a libdfi ticket with this info.
--
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: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org