Conversation
Signed-off-by: Jose Nino <[email protected]>
Signed-off-by: Jose Nino <[email protected]>
|
@Reflejo deliverables we talked about this morning. ptal when you have a chance. |
.bazelrc
Outdated
|
|
||
| build:fat --ios_multi_cpus=i386,x86_64,armv7,arm64 --fat_apk_cpu=x86,x86_64,armeabi-v7a,arm64-v8a | ||
|
|
||
| build:sizeopt -c opt --copt -Os --copt=-ggdb3 --linkopt=-fuse-ld=lld No newline at end of file |
There was a problem hiding this comment.
I don't think lld would be compatible with the iOS build?
There was a problem hiding this comment.
Can this also just get rolled into the release PR that @rebello95 is working on instead?
There was a problem hiding this comment.
@keith does iOS use gold?
I can work with @rebello95 to consolidate. I do think that for release we want to do --strip=never, while here we want to keep symbols and manually strip after bazel.
There was a problem hiding this comment.
Apple has their own linker, ld64, and we have to use that one
There was a problem hiding this comment.
I see, thanks for the info. My idea with the sizeopt config was to use to get binaries for size analysis, not necessarily for release. So the linker stipulation here is just for standardization of the tooling used in that type of investigation.
I'll adjust the bazel config accordingly once we settle on the release config.
There was a problem hiding this comment.
This would likely just mean that we wouldn't be able to use --config=sizeopt with iOS, right now @rebello95's open PR is using that config in the release mode
There was a problem hiding this comment.
gotcha, ok. I just looked at @rebello95's PR. I think I was confused about the ordering here. Now I see that #155 depends on this. I have fixed in a way that I think makes everything work together lmk what you think @keith
Signed-off-by: Jose Nino <[email protected]>
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Awesome! This is great work.
Can you please also provide notes on how you setup your ARM machine/instance/OS if there was anything interesting there? Or is it all standard Linux? If so maybe just note that and maybe reference which AWS machine you had success with if someone wants to try it out?
.bazelrc
Outdated
|
|
||
| build:fat --ios_multi_cpus=i386,x86_64,armv7,arm64 --fat_apk_cpu=x86,x86_64,armeabi-v7a,arm64-v8a | ||
|
|
||
| build:sizeopt -c opt --copt -Os --copt=-ggdb3 --linkopt=-fuse-ld=lld No newline at end of file |
There was a problem hiding this comment.
Can this also just get rolled into the release PR that @rebello95 is working on instead?
Signed-off-by: Jose Nino <[email protected]>
Signed-off-by: Jose Nino <[email protected]>
Signed-off-by: Jose Nino <[email protected]>
Signed-off-by: Jose Nino <[email protected]>
Signed-off-by: Jose Nino <[email protected]>
Signed-off-by: Jose Nino <[email protected]>
Signed-off-by: Jose Nino <[email protected]>
junr03
dismissed
mattklein123’s stale review
I addressed all of the review's comments
6 participants
Signed-off-by: Jose Nino [email protected]
Description: This PR concludes binary size investigation slated for issue #17. The three deliverables of this PR are:
perf/sizelabel.Additionally #181 will add CI jobs to add size regression analysis on every PR.
Risk Level: low - add new bazel target and docs.
Docs Changes: added developer documentation.
Fixes #17