Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Effectfully/inline fix #6842

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Effectfully/inline fix #6842

wants to merge 3 commits into from

Conversation

effectfully
Copy link
Contributor

This shows what happens if we add #6839 to #6841 (i.e. the base branch is zliu41/inline--fix, not master). See the comments.

@effectfully effectfully added Do not merge EXPERIMENT Experiments that we probably don't want to merge No Changelog Required Add this to skip the Changelog Check optimization labels Feb 13, 2025
@effectfully effectfully self-assigned this Feb 13, 2025
Comment on lines 0 to 2
({cpu: 397650662016
| mem: 1503390947}) No newline at end of file
({cpu: 409445366016
| mem: 1577107847})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+3% CPU, +5% MEM

395
Copy link
Contributor Author

@effectfully effectfully Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... but smaller size. And the best part is that the size even smaller than without inlining fix at all, i.e. this is smaller than on master.

Here's a three-way merge screenshot of CPU/MEM/size for "8 queens" (first @zliu41's branch, then master, then this branch):

Screenshot from 2025-02-13 02-01-41

Comment on lines 0 to 2
({cpu: 5476180
| mem: 28020}) No newline at end of file
({cpu: 5428180
| mem: 27720})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A slight cost reduction.

Comment on lines 0 to 2
({cpu: 304890223
| mem: 1188386}) No newline at end of file
({cpu: 304602223
| mem: 1186586})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a slight reduction

Comment on lines 0 to 2
({cpu: 88899135
| mem: 351394}) No newline at end of file
({cpu: 88611135
| mem: 349594})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a slight reduction

Comment on lines 0 to 2
({cpu: 174115055
| mem: 815970}) No newline at end of file
({cpu: 174099055
| mem: 815870})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then a bunch of reductions.

Comment on lines 0 to 2
({cpu: 243481580
| mem: 1195798}) No newline at end of file
({cpu: 243561580
| mem: 1196298})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then increases again.

Comment on lines 0 to 1
(\xxx -> 1) ((\s -> s s) (\s -> force trace "hello" (\z -> s s z))) No newline at end of file
1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A semantics change.

Comment on lines 0 to 2
({cpu: 5252180
| mem: 26620}) No newline at end of file
({cpu: 5300180
| mem: 26920})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A slight increase.

Comment on lines 1 to 9
(program
1.1.0
((\s -> s s)
(\s ds -> force (case ds [(delay 42), (\x xs -> delay (s s xs))]))
((\x0 ->
(\s -> s s)
(\s ds -> force (case ds [(delay 42), (\x xs -> delay (s s xs))]))
x0)
((\s -> s s)
(\s n ->
force
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I guess show the deficiency of the approach. It's nice that we can inline recursive functions better, but we still can't inline fully applied recursive functions.

Base automatically changed from zliu41/inline--fix to master February 13, 2025 16:36
@effectfully effectfully requested a review from zliu41 February 13, 2025 16:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Do not merge EXPERIMENT Experiments that we probably don't want to merge No Changelog Required Add this to skip the Changelog Check optimization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants