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

Jump to bottom

Exclude class method names from the opt-compiled binary #7225

Closed
vicuna opened this issue Apr 12, 2016 · 3 comments
Closed

Exclude class method names from the opt-compiled binary #7225

vicuna opened this issue Apr 12, 2016 · 3 comments
Assignees
@garrigue
Labels
feature-wish middle-end Stale

Comments

@vicuna
Copy link

vicuna commented Apr 12, 2016 •
edited by nojb

Original bug ID: 7225
Reporter: @db4
Assigned to: @garrigue
Status: assigned (set by @mshinwell on 2016-12-12T15:11:59Z)
Resolution: open
Priority: normal
Severity: feature
Version: 4.02.3
Category: middle end (typedtree to clambda)
Tags: patch
Monitored by: @gasche @hcarty

Bug description

Long time ago I asked in the mailing list about class method names in the opt-compiled binary [1]. It turned out that they are not necessary so then I made an internal patch to get rid of them. It does not improve performance, but excludes unnecessary data from binaries (e.g. size of lablgtk is decreased by 20%). The patch is attached, maybe you'll find it useful. Note the making "bootstrap" target is necessary while building OCaml compiler.

The path should now break anything. The only library I know that should be modified is batteries but the fix is trivial (sync batOo.mli with CamlinternalOO.mli)

[1] http://caml.inria.fr/pub/ml-archives/caml-list/2007/12/8ce5192907e067c310b658ad9bab8105.en.html

File attachments
@vicuna
Copy link
Author

vicuna commented Dec 12, 2016

Comment author: @mshinwell

Jacques, would you be able to either review this or determine that it should not be merged?

@vicuna
Copy link
Author

vicuna commented Dec 13, 2016

Comment author: @garrigue

Sorry for missing this patch.

The reduction in code size is attractive.
However, we must be careful that this introduces a change in semantics concerning instance variables and private methods : originally, only public methods were not allowed to have conflicting hashes, but with this patch this is extended to these 2 other categories.

So this requires some discussion. If this is the way to go, we may still want to rewrite the transclass part of the patch, to make it more readable.

@github-actions
Copy link

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 Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@garrigue garrigue

Labels
feature-wish middle-end Stale
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@garrigue @vicuna