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

Re-enable rr for Pkg tests #43521

Open
Keno opened this issue Dec 22, 2021 · 8 comments
Open

Re-enable rr for Pkg tests #43521

Keno opened this issue Dec 22, 2021 · 8 comments
Labels
ci Continuous integration libgit2 The libgit2 library or the LibGit2 stdlib module upstream The issue is with an upstream dependency, e.g. LLVM

Comments

@Keno
Copy link
Member

Keno commented Dec 22, 2021

@DilumAluthge notes that rr is currently disabled when running the Pkg tests on CI because libgit2 blows up rr trace file size due to excessive syscall traffic. This is an upstream libgit2 issue: libgit2/libgit2#6038. Opening this hear to record the knowledge and track the action item of updating the CI config once upstream libgit2 is fixed.

Our configuration:

linux 64_g3_st g3 . 64 x86_64 . . . 3 . yes . v4.8 2a058481b567f0e91b9aa3ce4ad4f09e6419355a

@Keno Keno added upstream The issue is with an upstream dependency, e.g. LLVM libgit2 The libgit2 library or the LibGit2 stdlib module ci Continuous integration labels Dec 22, 2021
@vtjnash
Copy link
Member

vtjnash commented Dec 22, 2021

Can we build libgit2 with mmap disabled?

@Keno
Copy link
Member Author

Keno commented Dec 22, 2021

Potentially. Reading the upstream discussion, it appears that this used to be done with read/write but was changed to work around coherence issues mmaping network drives on Windows. So hopefully changing this back should be easy, though I don't think there's a build flag for it or anything.

@Keno
Copy link
Member Author

Keno commented Dec 22, 2021

Actually, libgit2/libgit2#6039, may have already done exactly that. So we should probably try building with that and seeing if the issue is resolved.

@giordano
Copy link
Contributor

That seems to be in 1.3.0: JuliaPackaging/Yggdrasil#3962

@vtjnash
Copy link
Member

vtjnash commented Dec 22, 2021

Yeah, my suggestion was they should completely eliminate use of mmap everywhere, since read/write are usually much more optimal and reliable, particularly once adding threads. Let's see how it goes with that PR to start.

@vtjnash vtjnash closed this as completed Sep 28, 2022
@vtjnash
Copy link
Member

vtjnash commented Sep 28, 2022

We could still build with NO_MMAP, but it seems that this is already tracked upstream in libgit2 (libgit2/libgit2#6038), so we don't need to track it here

@Keno Keno reopened this Dec 12, 2024
@Keno
Copy link
Member Author

Keno commented Dec 12, 2024

We've had to re-disable Pkg tests on CI. A brief investigation shows that the primary source of syscall traffic is a 20 byte read done using a full mmap/munmap inside git_packfile_unpack_header. I think this could be fixed in libgit2, but for now just writing down the observation.

@Keno Keno added this to the 1.12 milestone Dec 12, 2024
@KristofferC
Copy link
Member

This doesn't feel like it has to be on the milestone. Can be reenabled whenever libgit2 has better support for rr but I don't see why we should tie that to our release.

@KristofferC KristofferC removed this from the 1.12 milestone Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous integration libgit2 The libgit2 library or the LibGit2 stdlib module upstream The issue is with an upstream dependency, e.g. LLVM
Projects
None yet
Development

No branches or pull requests

4 participants