Skip to content

Fix #1297, reduce memory usage from WriterT#1572

Merged
paf31 merged 2 commits into
masterfrom
1297
Oct 26, 2015
Merged

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

Conversation

@paf31

@paf31 paf31 commented Oct 26, 2015

Copy link
Copy Markdown
Contributor

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

The test suite hovers around ~1M usage 😄

@jdegoes

jdegoes commented Oct 26, 2015

Copy link
Copy Markdown

👍 👍 👍 🎉 🎉 🎉 🎆 🎆 🎆

@garyb

garyb commented Oct 26, 2015

Copy link
Copy Markdown
Member

Wonderful news! 💯

@paf31

paf31 commented Oct 26, 2015

Copy link
Copy Markdown
Contributor Author

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

paf31 commented Oct 26, 2015

Copy link
Copy Markdown
Contributor Author

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

Thanks!

@jdegoes

jdegoes commented Oct 26, 2015

Copy link
Copy Markdown

👍 Seems straightforward.

@paf31

paf31 commented Oct 26, 2015

Copy link
Copy Markdown
Contributor Author

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

paf31 commented Oct 26, 2015

Copy link
Copy Markdown
Contributor Author

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

@paf31

paf31 commented Oct 26, 2015

Copy link
Copy Markdown
Contributor Author

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

@paf31

paf31 commented Oct 26, 2015

Copy link
Copy Markdown
Contributor Author

Fixed, was a small issue with MonadBaseControl.

@paf31

paf31 commented Oct 27, 2015

Copy link
Copy Markdown
Contributor Author

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

@garyb

garyb commented Oct 27, 2015

Copy link
Copy Markdown
Member

Sure thing!

@paf31

paf31 commented Oct 27, 2015

Copy link
Copy Markdown
Contributor Author

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

@paf31

paf31 commented Oct 27, 2015

Copy link
Copy Markdown
Contributor Author

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

paf31 commented Oct 27, 2015

Copy link
Copy Markdown
Contributor Author

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

@garyb

garyb commented Oct 27, 2015

Copy link
Copy Markdown
Member

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

garyb commented Oct 27, 2015

Copy link
Copy Markdown
Member

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

@paf31

paf31 commented Oct 27, 2015

Copy link
Copy Markdown
Contributor Author

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