-
Notifications
You must be signed in to change notification settings - Fork 191
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: allow passing rayon threads when building aztec images #9096
Conversation
do not do parallel vk generation for mock circuits
Changes to public function bytecode sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
yarn-project/Earthfile
Outdated
@@ -36,7 +36,7 @@ build: | |||
BUILD ../barretenberg/cpp/+build | |||
BUILD ../barretenberg/ts/+build | |||
BUILD ../noir/+nargo | |||
BUILD ../noir-projects/+build | |||
BUILD ../noir-projects/+build --RAYON_NUM_THREADS=$RAYON_NUM_THREADS |
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.
damn it really be like that huh
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.
It shouldn't be
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.
Ah yeah I guess they are only automatically passed in the same earthfile, annoying caveat
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.
Might be cleaner to use --pass-args
You still need to set |
for pathname in "./target"/*.json; do | ||
BB_HASH=$BB_HASH node ../scripts/generate_vk_json.js "$pathname" "./target/keys" | ||
done | ||
fi | ||
|
||
for job in $(jobs -p); do | ||
wait $job || exit 1 |
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.
Do we need this when running sequentially?
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.
No, indeed. I missed this bit. Shouldn't hurt though.
@@ -56,11 +56,10 @@ build-mock-protocol-circuits: | |||
|
|||
ENV RAYON_NUM_THREADS=$RAYON_NUM_THREADS | |||
RUN echo "building with num threads $RAYON_NUM_THREADS" | |||
RUN cd mock-protocol-circuits && BB_HASH=$bb_source_hash NARGO=nargo ./bootstrap.sh | |||
RUN cd mock-protocol-circuits && BB_HASH=$bb_source_hash NARGO=nargo PARALLEL_VK=false ./bootstrap.sh |
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.
We never want to build in parallel here?
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.
No. For some reason, when it is in container (bootstrap_docker or earthly) parallel generation gives non-deterministic errors.
also, do not do parallel vk generation for mock circuits