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

[github patch] add an -opaque option to produce optimization-empty .cmx file for separate native compilation #6389

Closed
vicuna opened this Issue May 3, 2014 · 6 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

vicuna commented May 3, 2014

Original bug ID: 6389
Reporter: @gasche
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2016-12-07T10:34:37Z)
Resolution: fixed
Priority: low
Severity: feature
Fixed in version: 4.02.0+dev
Category: back end (clambda to assembly)
Tags: github, patch
Related to: #4454 #5859

Bug description

#30

The intent of this option is to enable semi-separate native
compilation. Currently, when a module changes, all the modules that
depend on it need to be recompiled. Activating -opaque disables
cross-module optimization, but should let us change a module without
having to recompile all the others (although to be honest this still
have to be verified).

This would be of practical value for development cycles of project
that have (compile;test) loops: currently compilation is much slower
in native code than in bytecode, but tests can be sensibly faster in
native code. -opaque could bring the best of both worlds, with fast
compiles and rather-fast tests.

Additional information

Reviewers noted that the "does not recompile dependencies when implementation changes" aspect only works for build-system that are content-based rather than timestamp-based, as recompiling the .cmx will update its modification time. I think this is fine, and that changing the compilation process to please timestamp-based system is an orthogonal question that should be discussed separately.

File attachments

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 4, 2014

Comment author: @alainfrisch

I'm in favor of accepting the patch, but it would be quite good to at least verify that it fulfills its goal (e.g. with ocamlbuild or with omake default rules).

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 6, 2014

Comment author: @alainfrisch

I've done a test on LexiFi's code base. For a given source file change and build target, compilation takes 24s without -opaque and 16s with -opaque. The gain is not huge, but we also need to link many executables (and this cannot be avoided!) and omake itself takes some time to parse and evaluate OMakefiles and check timestamp/digests (about 5s).

I assume that for some other scenarios, the gain would be more important.

Gabriel: since nobody else objected to the feature and it seems harmless, I think you can commit your patch on the trunk.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 6, 2014

Comment author: @gasche

Thanks a lot! I was busy with other things and kind of left this one by the wayside. I agree that will be a nice addition.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Jul 23, 2014

Comment author: @alainfrisch

Ping? Since the patch cannot break anything, I'd even push for putting it in 4.02.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Aug 7, 2014

Comment author: @gasche

I just rebased the patch against the current trunk. I am about to commit it -- only for now, but I'll check with Damien that it can go in 4.02 as well.

Currently the -opaque option is only supported by ocamlopt. Do you think it should be supported (and ignored) by ocamlc as well? I see you did a similar change for -for-pack.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Aug 19, 2014

Comment author: @gasche

Merged, with special authorization from our dear release manager, in 4.02@15106.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.