Skip to content

Use -O2 not -O3 for building flambda2 #333

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

Closed
wants to merge 1 commit into from

Conversation

mshinwell
Copy link
Collaborator

The recent change to make -O3 more aggressive has slowed build times down for flambda2 significantly (+10 minutes for the CI checks). I think -O2 is sufficient for this part of the code for the moment.

@mshinwell mshinwell added the flambda2 Prerequisite for, or part of, flambda2 label Oct 13, 2021
@poechsel
Copy link
Contributor

poechsel commented Oct 13, 2021

Build time (measured by executing time make -j 16 from a clean state):

For a compiler without #333 and #332 (as it was before)

real	3m13.943s
user	9m41.184s
sys	2m56.065s

For a compiler with #332

real	9m46.765s
user	20m29.501s
sys	3m12.723s

For this PR

real	6m11.362s
user	14m37.577s
sys	3m4.446s

@lpw25
Copy link
Collaborator

lpw25 commented Oct 13, 2021

Doesn't this suggest that O3 is now too aggressive?

@poechsel
Copy link
Contributor

We're currently looking at this a bit more in depth but it mostly seems to indicate that something might be wrong with the inlining depth.

I've benchmarked the time stage2 was taking for different values of inlining_depth:

> 2:
real	1m16.495s
> 4: 
real 1min29s
> 5: 
real	2m3.569s
> 6:
real	3m36.364s
> 10:
real	4m23.641s

Or in a graph:
study-stage2

With an inlining depth of 6 for -O2 it looks like:

  • simplify_binary_primitive.ml takes 90s to compile
  • flambda_kind.ml takes 14s to compile
  • numeric_types.ml takes 20s to compile

@mshinwell mshinwell marked this pull request as draft October 14, 2021 09:16
@mshinwell mshinwell closed this Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flambda2 Prerequisite for, or part of, flambda2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants