- User Since
- Jun 8 2014, 1:59 AM (224 w, 1 d)
Fri, Sep 21
Thu, Sep 20
Looks great, code is cleaner too! Thanks for doing this.
I would make a version of overwritingClosure that takes the size parameter and call that. It looks like we're missing out on the call to LDV_recordDead() that overwritingClosure also does.
Can we just turn off -Waggregate-return instead? It's not useful, there's absolutely nothing wrong with returning structs from functions.
Tue, Sep 18
Thanks for testing @osa1 and for the code review, and sorry (once again) for the collateral damage caused by the SRT rework!
Sat, Sep 15
Ok cool. We better tell @ekmett that stable-maps is not type-safe any more :)
Fri, Sep 14
@bgamari I see you reverted the SRT patches. So we're not going with this?
Thu, Sep 13
nofib results (ignore the runtime results which are unreliable on my laptop, this was just to confirm there was no increase in binary sizes).
Also fix this bug in the case of SRTs for recursive groups. See (yet
more) comments for details.
Wed, Sep 12
Yeah, it seems this doesn't completely fix the problem. Will figure out the right fix tomorrow.
@bgamari this validated locally but there's a perf test failure in Harbourmaster. Also I see trunk has been failing for quite a while, although the most recent few failures look like a different perf test. Yuck :(
Tue, Sep 11
Mon, Sep 10
Ok, could you also remove the hack in rts/package.conf.in since that's no longer necessary?
As a general point, if dtrace is to be a first-class supported feature then it needs to have tests and documentation.
Yeah, I find this useful too and often enable it in my build.mk.
Fri, Sep 7
I think we can go ahead with this. @bgamari would you like to push it?
I presume this is because we can't use the same method that the old build system uses? https://phabricator.haskell.org/diffusion/GHC/browse/master/rts%2Fpackage.conf.in$177-181
Thu, Sep 6
It would be nicer to put unrelated changes in separate commits, but OK.
If this adds a big compile time overhead, perhaps it should be enabled for -O2 only?
@lelf I'm not sure if you're raising an exception to this patch or not. If you are, could you explain exactly what problem it could introduce? @last_g says the patch has no effect on dtrace, do you disagree?
Good catch! Let's get this merged into the stable branch too.
Tue, Aug 28
Hmm, not sure - firstly there are probably a lot more places that need to be changed, and secondly is it worth it?
Sure, why not.
I don't have any strong opinions about dtrace (I've never used it). But if it's already broken, did anyone notice? Do we have a ticket for the existing breakage? Who would be affected by breaking it further?
If the results look good I think we can go with this.
Aug 23 2018
I think this is OK, but needs a comment.
Any functionality without a test case is effectively doomed to break again in a future release, but all things being equal I suppose it's better to have it fixed in this release :)
Aug 19 2018
One concern I have here is that changing the O(1) incremental expansion to a O(n) growth operation means that we have an unbounded unintnerruptible delay during compaction. This means that a compaction operation might delay GC arbitrarily, blocking all threads until the growth is complete.
Could we have a test too please?
Aug 11 2018
Aug 10 2018