Skip to content

Use -O2 not -O3 for building flambda2#333

Closed
mshinwell wants to merge 1 commit intooxcaml:mainfrom
mshinwell:flambda2-use-o2
Closed

Use -O2 not -O3 for building flambda2#333
mshinwell wants to merge 1 commit intooxcaml:mainfrom
mshinwell:flambda2-use-o2

Conversation

@mshinwell
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

lpw25 commented Oct 13, 2021

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

@poechsel
Copy link
Copy Markdown
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