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

[minor] Lifting constant tuples prevents static call optimization #7541

Closed
vicuna opened this issue May 25, 2017 · 2 comments
Closed

[minor] Lifting constant tuples prevents static call optimization #7541

vicuna opened this issue May 25, 2017 · 2 comments

Comments

@vicuna
Copy link

vicuna commented May 25, 2017

Original bug ID: 7541
Reporter: @gasche
Assigned to: @chambart
Status: feedback (set by @chambart on 2017-06-01T16:34:02Z)
Resolution: open
Priority: low
Severity: tweak
Version: 4.04.1
Category: back end (clambda to assembly)
Monitored by: @ygrek

Bug description

Consider:

let f (x, y) = ignore (x + y)
[@@inline never]

let () = for i = 1 to N do
f (1, 2);
done

let () = for i = 1 to N do
f (i, 3);
done

You would expect the first loop, calling the functions with statically-known arguments, to be as fast or faster as the second one, calling the function with a non-statically-known argument. With the non-flambda backend, the reverse happens: the second loop is sensibly faster than the first one. For one choice of N it runs in 1.19s instead of 1.59s on my machine -- so the first loop is 33% slower.

My understanding is that the constant tuple (1, 2) is turned into a module-global constant, which prevents the optimizer from doing the arity-raising optimization on the static call to f.

All non-flambda OCaml versions I have tested are affected, but flambda correctly optimizes this code. I don't know if this has a performance impact, so I only marked this as a "tweak" bug, but I found it surprising so I created a ticket to preserve the information.

Steps to reproduce

let rec f (x, y) = ignore (x + y)
[@@inline never]

let () =
let static_total = ref 0. in
let dynamic_total = ref 0. in
for i = 1 to 50 do
let start_static = Unix.gettimeofday () in
for _j = 1 to 10_000_000 do
f (1, 2);
done;
let stop_static = Unix.gettimeofday () in
static_total := !static_total +. (stop_static -. start_static);
let start_dynamic = Unix.gettimeofday () in
for j = 1 to 10_000_000 do
f (j, 2);
done;
let stop_dynamic = Unix.gettimeofday () in
dynamic_total := !dynamic_total +. (stop_dynamic -. start_dynamic);
done;
Printf.printf "%f: static total\n%f: dynamic total\n%!"
!static_total !dynamic_total

@vicuna
Copy link
Author

vicuna commented Jun 1, 2017

Comment author: @chambart

Flambda generates itself the tuplifying stub, hence can inline it, while Closure has no way to distinguish the tuplified and untuplified versions. This is introduced in Cmmgen. It would be particularly tedious doing something specific for that case.

Unless you have examples where it really matters and flambda is not applicable, this will result in wontfix.

@github-actions
Copy link

github-actions bot commented May 8, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 8, 2020
@lpw25 lpw25 closed this as completed May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants