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.

Sign up for GitHub

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

Jump to bottom

Finish the switch to lru.0.3.0 #357

Closed
pqwy opened this issue Apr 17, 2019 · 8 comments
Closed

Finish the switch to lru.0.3.0 #357

pqwy opened this issue Apr 17, 2019 · 8 comments
Milestone
2.2.0

Comments

@pqwy
Copy link

pqwy commented Apr 17, 2019

One certain, one potential performance bug:

  • add needs a corresponding trim.
  • If the order is irrelevant, this fold should be a fold_k.

I know... sorry for the breakage!
@dinosaure
Copy link
Member

Done by #358, thanks!

@pqwy
Copy link
Author

pqwy commented Apr 17, 2019

Ordering in there is important? What's the typical size of window?

@dinosaure
Copy link
Member

Yes, it's an heuristic used by git in general where we should apply duff/libXdiff with a better ratio with the most recently element added in the Window. size is 10 according to git.

@pqwy
Copy link
Author

pqwy commented Apr 17, 2019

The order of fold changed. Right?

@dinosaure
Copy link
Member

According to your CHANGES:

F.S.fold and F.S.iter iterate in LRU order.

If you mean that fold apply f to the most recent element firstly, this is what we want.

@pqwy
Copy link
Author

pqwy commented Apr 17, 2019

I'm not sure what's first/last here so it might be going in the wrong direction, but that's the right axis.

I think adding a test for this would be a good idea. Because if that ordering does matter, this code is either broken now, of has been until now.

@dinosaure
Copy link
Member

Ok, I will investigate and try to make a regression test of what I really expect about this part 👍 .

@dinosaure dinosaure mentioned this issue May 27, 2019
Closed
dinosaure added a commit to dinosaure/opam-repository that referenced this issue Jul 12, 2019
@dinosaure
[new release] git-http, git, git-unix and git-mirage (2.1.0)
a9f32eb 
CHANGES:

- Move to the last version of `decompress` (@dinosaure, mirage/ocaml-git#366)
- Check order of entries in a tree object (bug found by @samoht, fixed by @dinosaure, mirage/ocaml-git#365)
- Use `mmap` package (@dinosaure, mirage/ocaml-git#347, mirage/ocaml-git#360)
- Update README.md (@tcoopman, @dinosaure, mirage/ocaml-git#337, mirage/ocaml-git#359)
- `trim` the window used to pack (@pqwy, @dinosaure, mirage/ocaml-git#357, mirage/ocaml-git#358)
- Use lastest version of `lru.0.3.0` (@pqwy, @dinosaure, mirage/ocaml-git#352, mirage/ocaml-git#356)
- Fix smart protocol (fixed by @clecat and @dinosaure, feedbacks from @hannesm)
 * Pull-request mirage/ocaml-git#351, mirage/ocaml-git#350, mirage/ocaml-git#338
 * Issues mirage/ocaml-git#335, mirage/ocaml-git#342, mirage/ocaml-git#346

   + regression tests was added (@dinosaure)
   + semantics about negociation was explained (@clecat)
   + end-to-end tests partially done (@hannesm)

- Remove `sexplib` dependency (@samoht, mirage/ocaml-git#349)
- Fix smart protocol to accept empty response from `ls-remote` (bug found by @hannesm, fixed by @dinosaure, mirage/ocaml-git#348)
- Add `io-page-unix` as dependency to tests `git-mirage` (@dinosaure, mirage/ocaml-git#345)
- Remove deprecated `Cstruct.add_len` (replaced by `ke`) (@dinosaure, mirage/ocaml-git#345)
- Use `Uri.user_info` to be able to be authentified by a service like GitHub (@linse, review by @dinosaure, mirage/ocaml-git#341, mirage/ocaml-git#343)
- avoid clash between `digestif.c` and `digestif.ocaml` implementation (same for `checkseum`)
 * remove implementation dependencies on `git-unix` and `git-mirage` (bug found by @hannesm and @linse, fixed by @dinosaure, mirage/ocaml-git#339)

   This update should be fixed by `dune`'s variants and `>= digestif.0.7.2` and `>= checkseum.0.1.0`

- **breaking-change** add `etmp` as already-allocated buffer to encode Git object (@dinosaure, mirage/ocaml-git#336)
 * add `ke.0.3` as new dependency
- consumed inputs for every entries in a tree (bug found by @zspicko, fixed by @dinosaure, mirage/ocaml-git#334)
@dinosaure dinosaure mentioned this issue Jul 12, 2019
Merged
@dinosaure dinosaure mentioned this issue Oct 18, 2019
Closed
@dinosaure dinosaure mentioned this issue Jun 16, 2020
Closed
@dinosaure dinosaure added this to the 2.2.0 milestone Jun 29, 2020
@dinosaure
Copy link
Member

lru is not used anymore by ocaml-git.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.2.0
Development

No branches or pull requests
2 participants
@dinosaure @pqwy