pijul_org / pijul

#366 Investigate performance and stability

Opened by Boomshroom, on March 13, 2019
Boomshroom commented on March 13, 2019

I'm not sure if it's just me, but Pijul has a tendency to hang my computer when recording large changes or pulling a large number of patches like during an initial clone. On particularly bad instances, I have to manually kill it, or even hard reset the entire machine, leaving the repository in an inconsistent state forcing me to restart from the beginning. On top of that, I see pijul crashing due to the signature being incomplete. (It's there, but pijul just gave up on trying to read it and panicked.) On a related note, I found that there are 10 instances of block_on in pijul/src/commands/remote.rs and tends to do things like downloading all the patches serially rather than downloading multiple at once to take advantage of the slow I/O.\r+ \r + I took a look at htop while it was unrecording patches in an attempt to get the repository into a consistent state and noticed that my RAM and swap were both maxed out with all of the CPUs practically idle. This leads me to suspect that either pijul is trying to hold a large number of large files in memory at once, or there's a memory leak somewhere.\r + \r + Pijul has the potential to be great; the proof is there and it could work (though the diff algorithm could use some work), but at the moment, it's acting as a bottleneck for users (or at least me) halting work. It would be a good idea to investigate where some of these problems are coming from and optimise what we can.

Boomshroom commented on March 13, 2019

One of the most common errors I get other than incomplete HTTP headers, is:\r +

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', libcore/option.rs:345:21\r+
note: Run with `RUST_BACKTRACE=1` for a backtrace.\r                                               +
```\r                                                                                              +
Usually when I see this, it's nigh impossible to recover.
Boomshroom commented on March 14, 2019

I was able to make most network-accessing functions async and immediately saw a massive speed-up. Unfortunately, the repo got corrupted when I tried merging the 2 commits that had come in afterwords.

lthms commented on March 14, 2019

This is awesome! Thanks a lot for this work.\r + \r + If you want a conflict-free mirror of pijul to try recording your patches, you can use the one I have on pijul.lthms.xyz.\r+ \r +

pijul clone https://pijul.lthms.xyz/pijul/pijul\r                                                                                                     +
```\r                                                                                                                                                 +
\r                                                                                                                                                    +
Should work and give you a conflict-free repo.
Boomshroom commented on March 14, 2019

Thanks! Some questions: do you mean the mirror merges anything that has no conflicts, or that it's a fork that rejects them when trying to record?

pmeunier commented on March 14, 2019

@Boomshroom (and @lthms, btw), I'm the one to blame for the conflicts of yesterday. There is one patch that needs to happen at some point, reformatting everything once and for all with rustfmt. I tried to record that one, but I did it using a broken version of pijul record, and that created conflicts.\r+ \r + In the future, this shouldn't happen anymore on our master branch. I'm in the process of fixing the Nest to check for conflicts when someone pushes (in addition to allowing only fast-forward pushes on the client side).\r + \r + Thanks for your work! One way to solve the conflicts easily is to:\r + \r +

  1. get back to the point where your changes work and Pijul compiles (maybe by unrecording).\r +
  2. apply cargo fmt on the repository.\r +
  3. clone pijul from the nest.\r +
  4. manually cp your files to the fresh clone.\r +
  5. pijul record in the fresh clone.\r +
Boomshroom commented on March 14, 2019

Essentially working out of tree. Makes sense. Thank you.\r + \r + In the case of adding asynchrony, should i use nightly's async await? Manually using combinators is soooo annoying, especially with Rust's lifetimes. And what about 2018 edition?

lthms commented on March 15, 2019

@Boomshroom to answer your question, the link I gave you is just a regular pijul repository where I push onto only when I know that it will not cause any conflicts. This is manually achieve.\r+ \r + As for which rust to use in Pijul: afaik, @pmeunier wants to stick to stable. The question to whether or note use Rust 2018 has not been raised yet, therefore not answer too.

pmeunier commented on March 15, 2019

In my experience, using the latest Rust stable is what causes the smoothest experience for both the maintainers and the newcomers. As odd as it may seem for us, the notion of latest stable seems hard enough to understand for non-Rustaceans, a number of discussions have been opened here in the past saying "Pijul is broken, it doesn't compile" just because they were two or three versions of Rust behind.\r+ \r + Sure, the async/await syntax looks all cool and shiny, but think about the major rewrite we'll have to do in six months when the Rust compiler team decides to change it.\r + \r + This is just my two cents, but I think Nightly is suited only to smaller projects. Writing the Nest was fun in the beginning, but became increasingly painful because of Nightly, as I had to spend a ludicrous amount of time following changes in the compiler, fixing deprecation warnings, finding versions of Rustc that would compile all the dependencies at the same time… instead of adding new features.\r + \r + I can assist with combinators and/or writing Future instances if you want, I have some experience with it. Also, I don't think the syntax will solve your lifetime problems. If you want to run stuff on a threadpool (which you probably do), you will have to decide where things are stored, how they're moved, etc.

lthms commented on March 26, 2019

Fyi, as far as I can tell, pijul is in a far better shape now, with recent annoyance being fixed by @pmeunier.\r+ \r + Would you consider trying to record your changes again? It would be awesome!

pmeunier commented on March 26, 2019

By the way, this makes me think that moving SSH-related parts of pijul/src/commands/remote.rs into a new crate could be useful, both to show examples of how to use Thrussh, and to make them more efficient. For instance, a "pool" of downloaders would be a pretty useful thing to have.