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

Use chg if available, to speed up Send/Receive times on Linux #350

Closed
rmunn opened this issue Oct 17, 2024 · 8 comments
Closed

Use chg if available, to speed up Send/Receive times on Linux #350

rmunn opened this issue Oct 17, 2024 · 8 comments
Assignees

Comments

@rmunn
Copy link
Contributor

rmunn commented Oct 17, 2024

Watching Send/Receive logs on Language Forge, it has become clear that a large part of the time it takes to do Send/Receive involves waiting for a Python process to start in order to run commands like hg log -r0 --template "{node}". #258 was an attempt at speeding this up by skipping some of those log commands when the result is already known, but it had to be conservative about what is cached and so not every hg log call is cached even if it could be.

A better way we could improve the speed of Send/Receives, at least on Linux, would be to use the chg binary instead of hg. This is an hg command-line tool written in C, that communicates with a Python Mercurial process that stays in RAM and doesn't have to spin up every time. On a recent test of a Send/Receive with no changes (of the sena-3 project), it took about 0.7 seconds each time to spin up a Python process, and that happened 36 times during the course of the S/R. 36 times at about 0.7 seconds each makes 25.2 seconds spent waiting for a Python process to spin up. The total Send/Receive time was 46.7 seconds, meaning 54% of the Send/Receive time was spent simply waiting for a new Python process to spin up, do a single operation, and exit. Switching to use chg instead would therefore cut Send/Receive time in half.

Unfortunately, there is no chg.exe binary available for Windows in the TortoiseHg distribution that we're using in Mercurial4Chorus, so we can't easily replicate this speedup for most of our installed users. But it will definitely speed things up for Language Forge, so I think it's worth doing.

@rmunn rmunn self-assigned this Oct 17, 2024
@rmunn
Copy link
Contributor Author

rmunn commented Oct 18, 2024

The chg binary isn't included in the Mercurial4Chorus build we do. However, the source that we use does have the chg source code in the contrib/chg folder. It's only four C files and includes a Makefile, so building it in GHA should be trivial. Porting it to Windows would be non-trivial, though, as the chg source assumes it's running on Linux (for example, it determines the path of the running binary by reading /proc/self/exe which won't exist on Windows). If we really want to, it should be possible to port chg.c to use Windows API calls, but it certainly won't be as simple as adding it to our Linux build. However, adding it to our GHA build will be simple, and should get enough performance gains that it's worth doing.

@rmunn
Copy link
Contributor Author

rmunn commented Oct 25, 2024

I've built chg (for Linux only) in Mercurial4Chorus now, and tried it out. Running four Send/Receive integration tests in the Lexbox repo, running them with hg took 220 seconds (3:40), and running them with chg took 181 seconds (3:01). That shaved off about 40 seconds out of a total of 220 seconds, an 18% speedup. Not as much as the theoretical maximum, but of course there was time spent waiting for the network and so on in those four tests, this wasn't just a single Send/Receive.

But this proves that the chg build works, so I'll go ahead and merge in the Mercurial4Chorus PR and start a Chorus PR to pull that version into Chorus. Then I'll run the Chorus unit tests and see what the speed difference is.

@rmunn
Copy link
Contributor Author

rmunn commented Oct 26, 2024

Using chg also shaves about 13% off the time to do CrdtMerge. A CrdtMerge of the sena-3 project when it doesn't exist locally yet (meaning it requires a S/R clone, a CRDT sync, then a regular S/R) goes from 48.6 seconds with hg to 41.9 seconds with chg. Not the spectacular improvement I was hoping to get, because there's a lot going on besides Mercurial startup times, but still a nice solid gain.

@rmunn
Copy link
Contributor Author

rmunn commented Oct 26, 2024

One reason why I'm not seeing the speedups I expected might be because chg isn't getting to keep an hg process around for long. I was checking the output of ps while running Chorus unit tests, and saw that hg serve kept getting new process IDs, meaning the hg serve process isn't staying and responding to multiple requests. I need to investigate the details of how chg starts an hg server, and when it gets torn down again. Maybe there's something I can do to keep hg serve up longer, thereby responding to more chg requests and not paying Python startup costs as often.

@rmunn
Copy link
Contributor Author

rmunn commented Oct 26, 2024

Well, the Chorus unit tests produced a much more spectacular result. Running only the tests that were runnable on Linux, I got the following:

with hg:

Test summary: total: 1010, failed: 0, succeeded: 966, skipped: 20, duration: 1833.6s

with chg:

Test summary: total: 1010, failed: 5, succeeded: 961, skipped: 20, duration: 820.1s

That's more than twice as fast (820.1 is 45% of 1833.6) — but with 5 test failures.

The 5 test failures all involved running ChorusMerge, and looked something like this:

  Error: ChorusMerge Error: Unable to load shared library 'kernel32.dll' or one of its dependencies. In order to help diagnose loading problems, consider using a tool like strace. If you're using glibc, consider setting the LD_DEBUG environment variable:
  /.../chorus/output/Debug/net8.0/runtimes/linux-x64/native/kernel32.dll.so: cannot open shared object file: No such file or directory
  /usr/lib/dotnet/shared/Microsoft.NETCore.App/8.0.10/kernel32.dll.so: cannot open shared object file: No such file or directory
  (... skip a bunch more places that it tried, but failed because Linux doesn't have a kernel32.dll at all ...) 
  at Chorus.FileTypeHandlers.text.LongToShortConverter.GetShortPathName(String path, StringBuilder shortPath, Int32 shortPathLength)
  at Chorus.FileTypeHandlers.text.LongToShortConverter.GetShortPath(String path) in /.../chorus/src/LibChorus/FileTypeHandlers/text/TextFileTypeHandler.cs:line 162
  at Chorus.FileTypeHandlers.text.TextFileTypeHandler.GetRawMerge(String oursPath, String commonPath, String theirPath) in /.../chorus/src/LibChorus/FileTypeHandlers/text/TextFileTypeHandler.cs:line 67
  at Chorus.FileTypeHandlers.text.TextFileTypeHandler.Do3WayMerge(MergeOrder order) in /.../chorus/src/LibChorus/FileTypeHandlers/text/TextFileTypeHandler.cs:line 58
  at ChorusMerge.Program.Main(String[] args) in /.../chorus/src/ChorusMerge/Program.cs:line 61

It's not surprising that LongToShortConverter.GetShortPathName failed on Linux; there's a //Todo: not gonna work in Linux comment right there in the source. What I don't know yet is why it was being called at all, when running the same tests with hg didn't call that method. I'll narrow that down. But for now, this looks promising in terms of speeding up Chorus.

@rmunn
Copy link
Contributor Author

rmunn commented Oct 29, 2024

Interesting: that "unable to load kernel32.dll" failure just showed up in a test run that wasn't using chg at all: https://github.com/sillsdev/chorus/actions/runs/11569852594/job/32204416729?pr=354#step:7:4980

So that might be an issue in existing code, completely unrelated to the chg change. Will try to track it down, and open a new issue if necessary.

@rmunn
Copy link
Contributor Author

rmunn commented Oct 29, 2024

New issue opened: #355 with PR #356 fixing it.

@hahn-kev
Copy link
Contributor

hahn-kev commented Dec 9, 2024

decided to close this as we ended up using an environment variable instead

@hahn-kev hahn-kev closed this as not planned Won't fix, can't repro, duplicate, stale Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants