Skip to content

Bundle and optimize#2219

Closed
jutaro wants to merge 51 commits intopurescript:masterfrom
mgmeier:master
Closed

Bundle and optimize#2219
jutaro wants to merge 51 commits intopurescript:masterfrom
mgmeier:master

Conversation

@jutaro
Copy link
Copy Markdown

@jutaro jutaro commented Jul 6, 2016

This is #2139 (#2111) resubmitted, because the target branch switched from 0.9 to master. Think I have fixed all commented issues.

It is still not clear to me what are the wishes regarding to command line options. The current state is described here:
The uncurry optimization option is off by default. It can be explicitly enabled with -O or --optimize and disabled with --no-optimize.
We added the --no-optimize reacting on a comment from hdgarrood, but paf31 commeted with: Let's just have one option. How can we resolve this?

@jutaro jutaro mentioned this pull request Jul 6, 2016
@coveralls
Copy link
Copy Markdown

coveralls commented Jul 6, 2016

Coverage Status

Coverage decreased (-1.7%) to 55.819% when pulling 05caeea on jutaro:master into 7174a75 on purescript:master.

@philipcunningham
Copy link
Copy Markdown

I really hate to add noise but I would love to be able to use this!

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Oct 12, 2016

@Filib For now I recommend building from @jutaro's branch if you need this optimization.

I would like to think about more optimizations generally after 1.0, including general inlining on the functional core output, but for now, I don't want to commit to a certain specific optimization in master. I'm also not sure what the long term fate of psc-bundle and the use of language-javascript will be, in light of the recent changes to dump the functional core.

@jutaro
Copy link
Copy Markdown
Author

jutaro commented Oct 12, 2016

@phil, we had a conversation about this recently. As you see it's tiresome
to update my branch now for more then half a year.
Why not integrate the optimization (and drop it if something better is
coming - I will be happy then).

Where can I learn what "the recent changes to dump the functional core"
means?

2016-10-12 18:51 GMT+02:00 Phil Freeman notifications@github.com:

@Filib https://github.com/filib For now I recommend building from
@jutaro https://github.com/jutaro's branch if you need this
optimization.

I would like to think about more optimizations generally after 1.0,
including general inlining on the functional core output, but for now, I
don't want to commit to a certain specific optimization in master. I'm
also not sure what the long term fate of psc-bundle and the use of
language-javascript will be, in light of the recent changes to dump the
functional core.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2219 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAlqz8Jt8jRMKIP3XiYaN8ZxMOIvUEM2ks5qzRAtgaJpZM4JF0C8
.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Oct 14, 2016

As you see it's tiresome to update my branch now for more then half a year.
Why not integrate the optimization (and drop it if something better is coming - I will be happy then).

I'm sorry that you find it tiresome. Others have worked on alternative backends have been maintaining forks for years in some cases, but I don't want to merge in changes which are not in the plan for 1.0 and which we might remove later. Also, there are no tests or documentation.

Where can I learn what "the recent changes to dump the functional core"
means?

Here is the PR which was merged recently: #2275, and the issue which it was based on: #876.

@hdgarrood
Copy link
Copy Markdown
Contributor

@paf31

I'm also not sure what the long term fate of psc-bundle and the use of language-javascript will be, in light of the recent changes to dump the functional core.

Are you suggesting that we might want to consider an uncurrying optimization operating on the core functional representation instead of the JS representation? I guess the core functional representation is going to be simpler than a JS AST so that seems like a good idea, and also, presumably, that would mean that other backends could benefit from it, not just the JS backend?

@garyb
Copy link
Copy Markdown
Member

garyb commented Nov 27, 2016

I think if we were going to do that it would probably be better to do so on the imperative core. CoreFn only has one argument functions and I think it should stay that way personally.

I'll work on reviving CoreImp soon actually. It was pretty much done at one point, I had just been working on moving all of the optimisations that could applied to it, but I guess that's not a requirement, we could still do them on the JS at first and move them over piecemeal once CoreImp is merged.

# Resolving Conflicts in:
#	psc-bundle/Main.hs
#	src/Language/PureScript/Bundle.hs
# Conflicts:
#	CONTRIBUTORS.md
#	app/Command/Bundle.hs
#	purescript.cabal
#	src/Language/PureScript/Bundle.hs
@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Sep 9, 2017

As noted above, this needs (lots of) discussion before we decide how to handle optimization during bundling, or if we want to do it there at all. My preference is to use the corefn output as the target for this sort of optimization, and I don't want to add more code to purs bundle which might be removed later at this point.

I'm going through and closing PRs which aren't in line with plans for 1.0. Thanks @jutaro and @mgmeier for taking the initiative here, and I'm sorry it didn't work out. I hope you find a place for this optimization elsewhere, either in a fork or as a corefn pass.

@paf31 paf31 closed this Sep 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants