Skip to content

Possible optimisation: delaying allocations #165

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

Open
stedolan opened this issue Aug 17, 2021 · 5 comments
Open

Possible optimisation: delaying allocations #165

stedolan opened this issue Aug 17, 2021 · 5 comments

Comments

@stedolan
Copy link
Contributor

Here is an optimisation that I would like the compiler to do, which I was somewhat surprised that none of Closure / Flambda1 / Flambda2 currently perform. Currently, this function:

let go c =
  let s = Some c in
  if c < 0 then None else s

compiles to this cmm:

(function camlTest__go_81 (c/83: val)
 (let s/84 (alloc 1024 c/83) (if (< c/83 1) 1 s/84)))

which unconditionally allocates. It would be better to substitute away the let, moving the allocation under the if, so that it only allocates when c < 0.

Some more realistic examples:

  • Local functions:

    let f x ys =
      let aux s = s + x in
      match ys with
      | [] -> 42
      | y :: ys -> ... aux ...

    Ideally, the closure aux would only be allocated in the _ :: _ branch.

  • Inlined functions:

    let[@inline] log_change ch = if !logging_enabled then log := ch :: !log
    let do_stuff n m = log_change (Do_stuff (n, m)); n + m

    Ideally, the allocation Do_stuff (n, m) would only occur when logging_enabled is true.

@lthls
Copy link
Contributor

lthls commented Aug 17, 2021

Flambda 2 (more precisely, the translation from flambda 2 to Cmm) used to do it, but we removed it as it messed with quite a few tests in the testsuite (and possibly some tests in the Jane Street code base too, though I don't remember the details).

I think there were also some plans for a precise analysis of the best place to box/unbox numbers; it's possible that we could fit regular allocations in the same framework. But I don't know how much progress has been done on this.

@mshinwell
Copy link
Collaborator

We started looking at partial dead code elimination which I think should fix this (and also the cases where unnecessary boxing occurs at loop exit points). We had to pause work on PDCE but will get back to it after the first production release.

If I understand correctly the substitutions that used to happen in the Flambda to Cmm translation would only take effect if the value was linearly used. The reason we stopped doing this is because people are writing tests that rely on exact placement of allocation. The debate here really comes down to what kind of effects you consider allocation to have.

@stedolan
Copy link
Contributor Author

If these tests are so sensitive to the exact placement of allocations, do they break with safepoints as well?

@mshinwell
Copy link
Collaborator

I wouldn't have thought so, from what I remember, but people will have to watch out for that in due course. I think I might backport the safepoints patch well in advance of moving to 4.13, in fact.

@Gbury
Copy link
Contributor

Gbury commented Aug 18, 2021

From what I remember, moving allocations like this had the unexpected result of breaking tests that try and check that some section of code does not allocate. (Technically, allocations have observable side-effects because of the functions in Gc which return the number of allocations done).

In cases where the move does not cross function application, i think that with PDCE we should be able to move such allocations (but @chambart would probably know better).

mshinwell added a commit to mshinwell/oxcaml that referenced this issue May 1, 2023
e3076d2 Unboxed types v1 (oxcaml#139)
e68c72d update HACKING.jst.adoc (oxcaml#165)
6c5197b Merge pull request oxcaml#166 from mshinwell/merge-flambda-backend-2023-04-28
0c3dcf9 Fix for ocamldoc
09b9e1c Fix for -zero-alloc-check
71e5e07 Compilation fixes after merge
bf66257 Merge flambda-backend changes
a2556fc Add `[%exclave]` support (oxcaml#51)
ebe9576 Add data race freedom proposal (oxcaml#161)
3f3fc49 Merge pull request oxcaml#159 from riaqn/merge-backend
6c635dc minor changes after merge
99a0d85 Merge flambda-backend changes
2642463 Include the modes of values in debugging information (oxcaml#153)
4ecc8a4 Remove i386 CI check (oxcaml#155)

git-subtree-dir: ocaml
git-subtree-split: e3076d2
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

4 participants