Skip to content

Fix #1297, reduce memory usage from WriterT#1572

Merged
paf31 merged 2 commits intomasterfrom
1297
Oct 26, 2015
Merged

Fix #1297, reduce memory usage from WriterT#1572
paf31 merged 2 commits intomasterfrom
1297

Conversation

@paf31
Copy link
Copy Markdown
Contributor

@paf31 paf31 commented Oct 26, 2015

This reduces memory usage to <100M on my machine when compiling Halogen from scratch.

The test suite hovers around ~1M usage 😄

@jdegoes
Copy link
Copy Markdown

jdegoes commented Oct 26, 2015

👍 👍 👍 🎉 🎉 🎉 🎆 🎆 🎆

@garyb
Copy link
Copy Markdown
Member

garyb commented Oct 26, 2015

Wonderful news! 💯

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 26, 2015

Note this changes the ordering of the transformers, so another subtle change is that we can now get warnings and errors together instead of one or the other.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 26, 2015

If someone could please review the code, I'll merge this and release 0.7.5.1.

Thanks!

@jdegoes
Copy link
Copy Markdown

jdegoes commented Oct 26, 2015

👍 Seems straightforward.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 26, 2015

Thanks John. Merging..

paf31 added a commit that referenced this pull request Oct 26, 2015
Fix #1297, reduce memory usage from WriterT
@paf31 paf31 merged commit 322292c into master Oct 26, 2015
@paf31 paf31 deleted the 1297 branch October 26, 2015 20:49
@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 26, 2015

Hmm, weird, CI passed but shouldn't have.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 26, 2015

Ugh, warnings are all showing up twice now, which I didn't notice before. Will fix...

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 26, 2015

Fixed, was a small issue with MonadBaseControl.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 27, 2015

@garyb I'll cut the 0.7.5.1 branch now, ok? It seems solid to me at least.

@garyb
Copy link
Copy Markdown
Member

garyb commented Oct 27, 2015

Sure thing!

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 27, 2015

Maybe let's give it one more day of testing, then put it on npm?

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 27, 2015

So the binary build is up and fine, but the GHC 7.6 build doesn't work because of the import of Data.Monoid. So, I'll push a 0.7.5.2 up to Hackage, but do we need to tag a release on GitHub for that? Does NPM build from source at all?

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 27, 2015

Ah, whatever, I'll just tag a 0.7.5.2 release when Travis finishes.

@garyb
Copy link
Copy Markdown
Member

garyb commented Oct 27, 2015

It doesn't... and unfortuantely I just got done with updating a v0.7.5-rc.3 release on npm (using 0.7.5.1) 😄

@garyb
Copy link
Copy Markdown
Member

garyb commented Oct 27, 2015

I just added a 0.7.5.1 Windows build too, will do the same for 0.7.5.2.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Oct 27, 2015

Ah well if it works, that's good. 0.7.5.1 and 0.7.5.2 are functionally the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants