- User Since
- Jun 8 2014, 1:59 AM (249 w, 4 d)
Jan 23 2019
The status is that we still need this at FB and we're maintaining it as a local patch for now. It needs some work to be pushed upstream (see above discussion) which we really should do...
Jan 21 2019
Changing the type of a function from GhcMonad g => g a to GHCi a or from GHCi a to InputT GHCi a will also more likely result in a change in callsite.
Dec 21 2018
We can grep for idCafInfo and see there are none!
Ah OK. Commented on Trac #16065.
I note that Trac #4450 had exactly the same diagnosis that we've just rediscovered for Trac #8611, 8 years ago. So to avoid this happening again for more benchmarks, why don't we just do +RTS -V0 for all the single-threaded benchmarks in nofib? That is, add it by default to all parts of nofib except nofib/smp and nofib/parallel.
Remove CafInfo from IdLabel entirely.
Pass the Name -> CafInfo mapping directly to CmmBuildnfoTables; when you would do hasCaf lbl instead do hasCaf caf_env lbl:
Dec 18 2018
Dec 14 2018
As far as I can tell, in this diff we calculate the fingerprints as normal in mkIface but then update the CafInfo later. Isn't this wrong? The fingerprints need to take into account the CafInfo, so we have to defer computing the fingerprints until after the CafInfo is added.
Oh, and in general I think InputT has infested much more of the GHCi code than it really should, it would be nice to restrict it to a much smaller area.
I'd like to understand a bit more about the plan here. In general it's not necessary to overload everything in order to use the functions in a different monad - we can lift the functions instead. What is it that forces the overloading? Perhaps it's something to do with the structure of the interpreter, if we want to add more commands then the interpreter is calling back into our code, so we need to allow a different monad to be threaded through?
Yes, that's better
Dec 13 2018
Dec 10 2018
Actually I realised the Name is needed when fingerprinting so we couldn't make do with just the OccName, but I added a Note anyway.
Dec 7 2018
Yet another place where we want a way for the user to say whether they want to tune for size or speed, like gcc's -Os flag.
This makes zero difference to anything, because we land in the same switch branch: https://phabricator.haskell.org/diffusion/GHC/browse/master/rts%2Fsm%2FScav.c$1747-1753
Dec 5 2018
Yay. This code is so old!
Nov 30 2018
So neither fix is committable in its current state, if I'm understanding correctly? How would you like to move forward with this, do you need some help?
I think I would probably avoid the Bool argument by having a separate dynThunkHasCounter, but up to you.
This doesn't need a new WITH_FRAME type in the RTS, because we never need to treat these stack frames specially when walking the stack. There are a few examples of primitives that create new stack frames without adding a new frame types, e.g. maskAsyncExceptionszh_ret.
Nov 29 2018
Nov 26 2018
@osa1 did it get reverted?
Yes, good catch. It seems that we need something more refined than we have here.
Nov 23 2018
Do you have the nofib numbers for compilation time and allocations?
What impact does this have on compile time? I notice that you've enabled it at both -O and -O2.
I'm really happy to see the retainer profiler getting some love. Thanks!
Nov 22 2018
CmmSink is only run with -O, so perhaps we lose a little performance with -O0? You might argue that we don't care about that, but the goal of -O0 is to get as good code as possible with little effort. If we have dead binder info (even if it's partial) then we should use it.
Nov 21 2018
Nov 19 2018
The issue is that if a thread goes into a tight loop doing nothing but calls to makeStableName#, then without a call to MAYBE_GC() we will fill up the memory and never GC.
Oh, I'd completely forgotten about this patch. I'll see if we can get some numbers.
I think this is a general problem with unsafeDupablePerformIO that we can't do bracket-type things inside it. I'm thinking about possible solutions.
Oh, you also need to add a MAYBE_GC() call to the function.
Much simpler! allocate() is fine, it's used by lots of primops - in particular the Integer primops that need to allocate memory inside GMP use it, because we can't GC there either.
Let's do it.
Nov 17 2018
Good catch. But I think the fix is much too complicated - we could just call allocate() instead of ALLOC_PRIM(), which never fails. We'd also need a MAYBE_GC call at the beginning of the function.
Nov 16 2018
Nov 15 2018
Yep, all looks sensible to me.
Nov 14 2018
Doesn't this get set by the caller of newTask()?
Nov 12 2018
In this patch, I have copied the relevant bits from the GCC source code, and modified it to fit the GHC conventions.
But, we must make sure that we don't violate the GPL.
Nov 9 2018
If there's a better fix, please abandon this diff.
I think we'll need to do something much more general than this in the future when we have a mixture of Int8#, Int16#, etc. fields that need to be packed into words. Happy to go with this for now though.
Nov 5 2018
Trac Trac #15847: We don't support loading non-PIC .o on i386, this is why I noticed this issue
Nov 2 2018
I'm not sure I understand the problem here. Why is there a missing symbol error?
Sure, seems reasonable.
Oct 26 2018
Thanks for adding the test!
Oct 24 2018
This is great. It could be even better if
- We had your benchmark in the test suite, perhaps testsuite/tests/perf? We can use small parameters so it doesn't run for long.
- We point to the benchmark from comments in the code, so that others can use it when making future changes.
Oct 19 2018
A bit more review
Will this work with g++ with no flags? If not, what else would be needed?
Oct 18 2018
btw, use #1234 for ticket references in Phabricator markup.
I think we could remove this, it's a very small code-size optimisation and would require a fair bit more work to implement, not at all clear that it's worth it. I commented more on the ticket.
Oct 15 2018
- We should build these at installation time, so they don't have to go into the binary distribution (the same goes for the vanilla way` libHSfoo.o` files)
- This also needs to be done in Hadrian
Oct 10 2018
This looks OK.
Oct 9 2018
For Maybe [String] I would just pprDebug $ text (show x).
Oct 8 2018
Wow! Some questions below.
It's not obvious to me that we should render a String like this in Outputable, after all we don't render a FastString with double quotes, and we don't render a Char with single quotes. Outputable is not intended to produce Haskell concrete syntax, it's mainly for pretty-printing the various internal compiler data structures.
Oct 7 2018
Looks good. Was that with the libraries recompiled too?
Oct 6 2018
I'd be really interested to see what effect this has on performance and code size too.
Oct 4 2018
nofib results below
@osa1 We save the old info pointer when entering a CAF: https://phabricator.haskell.org/diffusion/GHC/browse/master/rts%2Fsm%2FStorage.c$406
Looks good to go, provided it passes validate.
Oct 3 2018
Oct 1 2018
Another option might be to keep top-level strings updatable, but make the GC revert the CAF. That way we would get some of the sharing back.