-
Notifications
You must be signed in to change notification settings - Fork 322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: add deployment usage in java quickstart #3220
Conversation
@@ -148,19 +148,19 @@ thirdparty-fast: | |||
if [ "$$new_zetasql_version" != "$(ZETASQL_VERSION)" ] ; then \ | |||
echo "[deps]: thirdparty up-to-date. reinstall zetasql from $(ZETASQL_VERSION) to $$new_zetasql_version"; \ | |||
$(MAKE) thirdparty-configure; \ | |||
$(CMAKE_PRG) --build $(THIRD_PARTY_BUILD_DIR) --target zetasql; \ | |||
$(CMAKE_PRG) --build $(THIRD_PARTY_BUILD_DIR) -j $(NPROC) --target zetasql; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does NPROC improve speed ? this is the parallelism of build all thirdparty deps, parallelism of each dep is handled inside each cmake/Fetch*.cmake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, speed faster. I can't find -j or other options in Fetch*.cmake, where?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zetasql uses bazel, which has its own parallelism strategy builtin, no need to specify NPROC ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can’t recall about zetasql, I'll revert this change, and check it when I build the old stdc++ capable image.
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #3220 +/- ##
=============================================
- Coverage 81.48% 75.60% -5.89%
- Complexity 0 393 +393
=============================================
Files 8 678 +670
Lines 578 125400 +124822
Branches 0 1181 +1181
=============================================
+ Hits 471 94809 +94338
- Misses 91 30355 +30264
- Partials 16 236 +220 see 670 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
@@ -148,7 +148,7 @@ thirdparty-fast: | |||
if [ "$$new_zetasql_version" != "$(ZETASQL_VERSION)" ] ; then \ | |||
echo "[deps]: thirdparty up-to-date. reinstall zetasql from $(ZETASQL_VERSION) to $$new_zetasql_version"; \ | |||
$(MAKE) thirdparty-configure; \ | |||
$(CMAKE_PRG) --build $(THIRD_PARTY_BUILD_DIR) --target zetasql; \ | |||
$(CMAKE_PRG) --build $(THIRD_PARTY_BUILD_DIR) -j $(NPROC) --target zetasql; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert here as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
already done 631f6ea, but seems we can't see it
close #2920