Skip to content
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

chore: experiment with only applying ASAN and LSAN to Magma C files #12113

Merged
merged 1 commit into from Mar 15, 2022

Conversation

themarwhal
Copy link
Member

@themarwhal themarwhal commented Mar 14, 2022

Signed-off-by: GitHub noreply@github.com

Summary

Just experimenting to see how the selective ASAN/LSAN enablement affects cache size

Master cache size after running bazel test //orc8r/gateway/c/... //lte/gateway/c/... --config=asan and bazel test //orc8r/gateway/c/... //lte/gateway/c/... --config=lsan: 13G
PR cache size after running bazel test //orc8r/gateway/c/... //lte/gateway/c/... --config=asan and bazel test //orc8r/gateway/c/... //lte/gateway/c/... --config=lsan: 8.4G

@nstng / @LKreutzer I think reducing ASAN/LSAN instrumentalization scope will help the cache size as well. (for CI, at least)

I've also found some repos in sourcegraph use per_file_copt to exclude dependencies from enabling it. https://sourcegraph.com/github.com/google/oss-fuzz/-/blob/projects/envoy/build.sh?L73:10

Test Plan

Additional Information

  • This change is backwards-breaking

@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines. label Mar 14, 2022
@github-actions
Copy link
Contributor

Thanks for opening a PR! 💯

A couple initial guidelines

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@themarwhal themarwhal force-pushed the omit-asan-instrumentalization branch from 07d8f5f to a174289 Compare March 14, 2022 21:22
@pull-request-size pull-request-size bot added size/XS Denotes a PR that changes 0-9 lines. and removed size/S Denotes a PR that changes 10-29 lines. labels Mar 14, 2022
@github-actions
Copy link
Contributor

agw-workflow

  52 files    79 suites   5m 12s ⏱️
891 tests 881 ✔️ 9 💤 0  1 🔥
894 runs  882 ✔️ 9 💤 0  3 🔥

For more details on these errors, see this check.

Results for commit a174289.

Copy link
Contributor

@LKreutzer LKreutzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally - in pairing with @nstng and @jheidbrink. Python tests still do not work with linkopt. There seems to be no feature to restrict linkopts to certain files.

@themarwhal themarwhal marked this pull request as ready for review March 15, 2022 17:09
@themarwhal themarwhal requested a review from a team March 15, 2022 17:09
@themarwhal themarwhal merged commit c2f73c3 into magma:master Mar 15, 2022
@themarwhal themarwhal deleted the omit-asan-instrumentalization branch March 15, 2022 17:09
ardzoht added a commit to ardzoht/magma that referenced this pull request Mar 16, 2022
* 'master' of github.com:magma/magma:
  chore: Mark nas_converter_test as manual until flakiness is addressed (magma#12130)
  chore(mme): address GH11898 (magma#12129)
  fix(agw): Update DNS resolvers for ec2 instance (magma#12045)
  feat(feg_relay): move session proxy to NH implementation (magma#11080)
  chore: Use per_file_copt for MME unit test flag (magma#12112)
  chore(mme): Remove unused trace defines (magma#12055)
  chore: experiment with only applying ASAN and LSAN to Magma C files (magma#12113)
  Revert "test(mme): Add injection of state loaded in S1AP state manager (magma#11456)" (magma#12121)
  chore: bump ssri (magma#12032)
  feat(dp): Add grant attempt count (magma#12101)
  fix(mme): Fix typing_extensions version dependency on magma_test (magma#12110)
ardzoht pushed a commit that referenced this pull request Mar 30, 2022
emakeev pushed a commit to emakeev/magma that referenced this pull request Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants