You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "lordgamez (via GitHub)" <gi...@apache.org> on 2025/04/25 15:28:23 UTC
[PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
lordgamez opened a new pull request, #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963
https://issues.apache.org/jira/browse/MINIFICPP-2563
-----------------
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963#discussion_r2197547240
##########
extensions/standard-processors/tests/unit/ManifestTests.cpp:
##########
@@ -69,7 +69,7 @@ TEST_CASE("Test Valid Regex", "[validRegex]") {
REQUIRE_FALSE(resp.children.empty());
const auto &processors = resp.children[0];
REQUIRE_FALSE(processors.children.empty());
- const auto &proc_0 = processors.children[0];
+ const auto &proc_0 = "org.apache.nifi.minifi.processors.UpdateAttribute" != processors.children[0].name ? processors.children[0] : processors.children[1];
Review Comment:
In this test we need a processor with properties and UpdateAttribute does not have any, I updated the name to `processor_with_properties` in https://github.com/apache/nifi-minifi-cpp/pull/1963/commits/f6c1da33f0910a2ce1d745744c1d3f3d05813774
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on PR #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963#issuecomment-2966115326
> do we have some numbers how this affects build time and the gains on runtime?
No, not at the moment. This is an advanced build option similar to the address sanitizer, which was only tested on linux at the moment. We can do some experiments/benchmarking with it later to see how could this be utilized in the future, maybe in releases or documenting when is it recommended to be used.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963#discussion_r2140298896
##########
libminifi/src/agent/agent_docs.cpp:
##########
@@ -20,15 +20,18 @@
namespace org::apache::nifi::minifi {
namespace {
-std::map<std::string, Components> AgentDocs_class_mappings;
+std::map<std::string, Components>& getAgentDocsClassMappings() {
+ static std::map<std::string, Components> mappings;
+ return mappings;
Review Comment:
Same issue with the static `mappings` map.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on code in PR #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963#discussion_r2189342326
##########
extensions/standard-processors/tests/unit/ManifestTests.cpp:
##########
@@ -69,7 +69,7 @@ TEST_CASE("Test Valid Regex", "[validRegex]") {
REQUIRE_FALSE(resp.children.empty());
const auto &processors = resp.children[0];
REQUIRE_FALSE(processors.children.empty());
- const auto &proc_0 = processors.children[0];
+ const auto &proc_0 = "org.apache.nifi.minifi.processors.UpdateAttribute" != processors.children[0].name ? processors.children[0] : processors.children[1];
Review Comment:
very minor, but maybe rename this to `update_attribute_processor`?
edit: or is it `not_update_attribute_processor`? what is it that all processors have except `UpdateAttribute`? could we look for a specific processor type instead of something-near-the-front-that-is-not-UpdateAttribute?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on code in PR #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963#discussion_r2189342326
##########
extensions/standard-processors/tests/unit/ManifestTests.cpp:
##########
@@ -69,7 +69,7 @@ TEST_CASE("Test Valid Regex", "[validRegex]") {
REQUIRE_FALSE(resp.children.empty());
const auto &processors = resp.children[0];
REQUIRE_FALSE(processors.children.empty());
- const auto &proc_0 = processors.children[0];
+ const auto &proc_0 = "org.apache.nifi.minifi.processors.UpdateAttribute" != processors.children[0].name ? processors.children[0] : processors.children[1];
Review Comment:
very minor, but maybe rename this to `update_attribute_processor`?
edit: or is it `not_update_attribute_processor`? what is it that all processors have except `UpdateAttribute`?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963#discussion_r2063907463
##########
extensions/rocksdb-repos/database/RocksDatabase.h:
##########
@@ -59,8 +59,6 @@ class RocksDatabase {
private:
const std::string column_;
std::shared_ptr<RocksDbInstance> db_;
-
- static std::shared_ptr<core::logging::Logger> logger_;
Review Comment:
what made this necessary? I support the refactor, I'm just interested in the reason, it's something we might need to take into account during code design in the future.
##########
libminifi/src/agent/agent_docs.cpp:
##########
@@ -20,15 +20,18 @@
namespace org::apache::nifi::minifi {
namespace {
-std::map<std::string, Components> AgentDocs_class_mappings;
+std::map<std::string, Components>& getAgentDocsClassMappings() {
+ static std::map<std::string, Components> mappings;
+ return mappings;
Review Comment:
same question here
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits closed pull request #1963: MINIFICPP-2563 Add build option for enabling link time optimization
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963#discussion_r2140298192
##########
extensions/rocksdb-repos/database/RocksDatabase.h:
##########
@@ -59,8 +59,6 @@ class RocksDatabase {
private:
const std::string column_;
std::shared_ptr<RocksDbInstance> db_;
-
- static std::shared_ptr<core::logging::Logger> logger_;
Review Comment:
When LTO is enabled the order of initialization of the static object changes and the logger_ was not available when it was first used, so it crashed.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on code in PR #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963#discussion_r2189342326
##########
extensions/standard-processors/tests/unit/ManifestTests.cpp:
##########
@@ -69,7 +69,7 @@ TEST_CASE("Test Valid Regex", "[validRegex]") {
REQUIRE_FALSE(resp.children.empty());
const auto &processors = resp.children[0];
REQUIRE_FALSE(processors.children.empty());
- const auto &proc_0 = processors.children[0];
+ const auto &proc_0 = "org.apache.nifi.minifi.processors.UpdateAttribute" != processors.children[0].name ? processors.children[0] : processors.children[1];
Review Comment:
very minor, but maybe rename this to `update_attribute_processor`?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2563 Add build option for enabling link time optimization [nifi-minifi-cpp]
Posted by "adamdebreceni (via GitHub)" <gi...@apache.org>.
adamdebreceni commented on PR #1963:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1963#issuecomment-2966091579
do we have some numbers how this affects build time and the gains on runtime?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org