- User Since
- Jun 8 2014, 1:59 AM (193 w, 5 d)
Good call, thanks for fixing this. cc @niteria
Seems reasonable. Eventually we'll want to use the proper Int16# (etc.) types here, when we have those.
Wed, Feb 21
I just tried to make FAST=YES in libraries/base and found that the makefile we used to have to support this is no longer there. This diff seems to be the culprit. I can work around it for now, but FYI this was useful!
Tue, Feb 20
Mon, Feb 19
LGTM, I'll take a 1% binary size reduction in exchange for <1% at compile time.
In general it sounds like a good plan, though I'm not familiar with the details of NT paths. I'm amazed and saddened at the lengths upper layers have to go to to work around the legacy backwards compatibility in lower layers though.
Thu, Feb 8
Wed, Feb 7
Mon, Feb 5
No objection here, so long as you can get the CI to pass.
Sun, Feb 4
Yeah, I'm suspicious of that change to CmmSink as I noted in the comment: https://phabricator.haskell.org/rGHC6d14c1485cb570cbd183bcdc0f858d9a6dc1eb31#inline-626
This looks OK to me.
Thu, Feb 1
I think it makes sense if we want to initialize cost centre fields correctly the first time, rather than putting a placeholder (noCCS) and then updating it.
If it passes the tests this should be OK. If it's missing on other 32-bit architectures we'll need to add it there too - I'm slightly surprised we got away without this for so long, how is it not needed by the Int64 library I wonder?
Thanks for taking care of this! Nothing is ever simple :-/
Tue, Jan 30
Is this still reverted?
Woah, I'm too late to the party to review this, but this is deeply suspicious:
Great! I think we're good to go here.
This looks OK, in fact I would have kept the CAF cost centre business separate instead of merging it with CoreToStg - sorry we probably weren't clear about this bit, it was only the recursive traversal of stgMassageForProfiling we wanted to get rid of. But it's OK to move it to CoreToStg.
Mon, Jan 29
I thought we decided to get rid of the STG traversal in stgMassageForProfiling? Or do you want to do that separately? I'm a little concerned about adding another complete traversal (albeit only when profiling, I know).
Jan 24 2018
By the way, is it important that I'm using minimal nursery size instead of actual one for calculations? How large can they grow?
And to answer your question,
Jan 23 2018
Nice, just one thing to fix.
So there's a bug somewhere? That's interesting! Does the program actually crash or just hang?
Jan 22 2018
Not sure about this. We already have code to do something similar here:
FYI, @bgamari will commit it, so you're done.
FYI, @bgamari will commit it, so you're done.
Looks great, thanks for making the test!
Jan 19 2018
Jan 18 2018
Jan 16 2018
Would you prefer I keep the current design or remove the redundancies?
Oops, thanks for the fix!
Jan 15 2018
I'm a bit concerned about the duplication between RTSSummaryStats and RTSStats. I suggest:
nubSort would be an improvement too.
Jan 12 2018
No objection in principle. Please fix the long lines and I can take a closure look at the diff.
Jan 10 2018
I definitely support bringing in those docs somewhere (with the fix I just added to https://ghc.haskell.org/trac/ghc/ticket/10840).
Just a thought for the future: on CmmCondBranch we have a likely flag which says which branch is more likely to be taken and we can use that to generate better code. If we had something similar for CmmSwitch we could use it to make different decisions here.
Jan 8 2018
The eventlog_enabled flag doesn't quite do what you want it to do, I think. It doesn't protect all the places where we emit events, for example see traceSchedEvent_. I'm not entirely sure what happened - I think eventlog_enabled was originally just to indicate whether we should create the file, but then it got used in some other places too.
That is indeed a help. But it sits in a larger context: the invariants about pointer tagging. Are these invariants documented anywhere (except in the paper)? Is there a wiki page?
Jan 5 2018
Dec 18 2017
smaller test output file
I like the idea. Can you put an example of the generated code in the test plan so we can check what's being generated?
Dec 15 2017
@Phyx The long-term plan would be to drop addDLL(), yes.
Overall this is great, we just need to work on the way to handle keepCAFs/retain_cafs.
Dec 13 2017
Dec 7 2017
Hah, I didn't notice how big it was. I'll fix it.
add test output
Could the remaining assertion (about compacts) be changed to produce a clearer error message?
Dec 4 2017
Dec 1 2017
Yes, I had considered binary search. The trouble is that this could be rather expensive since when we succeed in mapping a size, we will need to again unmap it to try the next larger guess. I haven't measured but I would expect a this mapping/unmapping will be much more expensive than simply attempting to mmap successively smaller sizes until we hit success.
Nov 30 2017
It feels like a bit of a hack
Nice catch, I think that's historical.
Nov 28 2017
Nov 27 2017
Mostly this looks reasonable to me, thanks for doing all this! One question inline.
Nov 22 2017
Could we query the ulimit value and use that to make an informed guess?
Whatever we do here should probably be consistent between GHCi and ghc --make, so that argues for moving this into GhcMake rather than having it be GHCi-specific.
Nov 20 2017
Nov 16 2017
Yup, I didn't think very hard about this in the recent refactor of this code. Looks reasonable to me.