| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
|
|
|
| |
[ci skip]
|
|
|
|
|
| |
This finally moves the newly generalised heap traversal code from the
retainer profiler into it's own file.
|
|
|
|
|
| |
A lot of these includes are presumably leftovers from when the retainer
profiler still did it's own heap profiling.
|
|
|
|
|
|
| |
Turns out some genius disabled warnings for RetainerProfile.c in the build
system. That would have been good to know about five silent type mismatch
crashes ago.. :)
|
| |
|
|
|
|
|
|
|
|
|
| |
Currently it is necessary for user code to expend at least one extra bit in
the closure header just to know whether visit() should return true or
false, to indicate if children should be traversed.
The generic traversal code already has this information in the visited bit
so simply pass it to the visit callback.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There's simply no need anymore for this whole business. Instead of
individually traversing roots in retainRoot() we just push them all onto
the stack and traverse everything in one go.
This feature was not really used anyways. There is an
`ASSERT(isEmptyWorkStack(ts))` at the top of retainRoot() which means there
really can't ever have been any chunks at the toplevel.
The only place where this was probably used is in traversePushStack but
only way back when we were still using explicit recursion on the
C callstack.
Since the code was changed to use an explicit traversal-stack these
stack-chunks can never escape one call to traversePushStack anymore. See
commit 5f1d949ab9 ("Remove explicit recursion in retainer profiling")
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
| |
STATIC_INLINE already does what the code wanted here, no need to duplicate
the functionality here.
|
|
|
|
|
|
| |
These invariants don't seem to make any sense in the current code. The
text talks about c_child_r as if it were an StgClosure, for which RSET()
would make sense, but it's a retainer aka 'CostCentreStack*'.
|
|
|
|
|
| |
A lot of comments and strings are still talking about old names, fix
that.
|
|
|
|
|
|
|
|
|
|
| |
Keeping track of the maximum stack seems like a good idea in all
configurations. The associated ASSERTs only materialize in debug mode but
having the statistic is nice.
To make the debug code less prone to bitrotting I introduce a function
'debug()' which doesn't actually print by default and is #define'd away
only when the standard DEBUG define is off.
|
|
|
|
|
| |
This gets all remaining functions in-line with the new 'traverse' prefix
and module name.
|
|
|
|
|
|
|
| |
Commit dbef766ce7 ("Profiling cleanup.") made this debug code obsolete by
removing the 'cost' function without a replacement. As best I can tell the
retainer profiler used to do some heap census too and this debug code was
mainly concerned with that.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In the old code when DEBUG_RETAINER was set, FIRST_APPROACH is
implied. However ProfHeap.c now depends on printRetainerSetShort which is
only available with SECOND_APPROACH. This is because with FIRST_APPROACH
retainerProfile() will free all retainer sets before returning so by the
time ProfHeap calls dumpCensus the retainer set pointers are segfaulty.
Since all of this debugging code obviously hasn't been compiled in ages
anyways I'm taking the liberty of just removing it.
Remember guys: Dead code is a liability not an asset :)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Instead of breaking out of the switch-in-while construct using `return` this
uses `goto out` which makes it possible to share a lot of the out-variable
assignment code in all the cases.
I also replaced the nasty `while(true)` business by the real loop
condition: `while(*c == NULL)`. All `break` calls inside the switch aready
have either a check for NULL or an assignment of `c` to NULL so this should
not change any behaviour.
Using `goto out` also allowed me to remove another minor wart: In the
MVAR_*/WEAK cases the popOff() call used to happen before reading the
stackElement. This looked like a use-after-free hazard to me as the stack
is allocated in blocks and depletion of a block could mean it getting freed
and possibly overwritten by zero or garbage, depending on the block
allocator's behaviour.
|
|
|
|
|
|
|
| |
This essentially turns the heap traversal code into a visitor. You add a
bunch of roots to the work-stack and then the callback you give to
traverseWorkStack() will be called with every reachable closure at least
once.
|
|
|
|
|
|
|
| |
This commit starts renaming some flip bit related functions for the
generalised heap traversal code and adds provitions for sharing the
per-closure profiling header field currently used exclusively for retainer
profiling with other heap traversal profiling modes.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This essentially ammounts to s/retainer/stackData/, s/c_child_r/data/ and
some temporary casting of c_child_r to stackData until refactoring of this
module is completed by a subsequent commit. We also introduce a new union
'stackData' which will contain the actual extra data to be stored on the
stack.
The idea is to make the heap traversal logic of the retainer profiler ready
for extraction into it's own module. So talking about "retainers" there
doesn't really make sense anymore.
Essentially the "retainers" we store in the stack are just data associated
with the push()ed closures which we return when pop()ing it.
|
|
|
|
|
| |
I don't see a point in having this live in 'info', just seems to make the
code more complicated.
|
|
|
|
|
|
|
|
|
|
| |
Global state is ugly and hard to test. Since the profiling code isn't quite
as performance critical as, say, GC we should prefer better code here.
I would like to move the 'flip' bit into the struct too but that's
complicated by the fact that the defines which use it directly are also
called from ProfHeap where the traversalState is not easily
available. Maybe in a future commit.
|
|
|
|
|
| |
This can only ever be one since 5f1d949ab9 ("Remove explicit recursion in
retainer profiling"), so it's pointless.
|
|
|
|
|
|
|
|
| |
When pop() returns with `*c == NULL` retainerProfile will immediately
return. All other code paths is pop() continue with the next stackElement
when this happens so it seems weird to me that TREC_CHUNK we would suddenly
abort everything even though the stack might still have elements left to
process.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Debug code have not been updated for a long time,
now it's changed to it compiles with recent RTS.
Reviewers: bgamari, erikd, simonmar
Reviewed By: simonmar
Subscribers: rwbarton, carter
Differential Revision: https://phabricator.haskell.org/D5369
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Retainer profiling contained a recursion that under
certain circumstances could lead to the stack overflow
in C code.
The idea of the improvement is to keep an explicit stack for the
object, more precise to reuse existing stack, but allow new type of
objects to be stored there.
There is no reliable reproducer that is not a big program
but in some cases foldr (+) 0 [1..10000000] can work.
Reviewers: bgamari, simonmar, erikd, osa1
Reviewed By: bgamari, osa1
Subscribers: osa1, rwbarton, carter
GHC Trac Issues: #14758
Differential Revision: https://phabricator.haskell.org/D5351
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
- Retainer profiler: init_srt_thunk() should mark the stack entry as SRT
- Retainer profiler: Remove an incorrect assertion about FUN_STATIC.
FUN_STATIC does not have to have an SRT.
- Fix nptrs of BCO
Test Plan: validate
Reviewers: simonmar, bgamari, erikd
Reviewed By: simonmar
Subscribers: rwbarton, carter
Differential Revision: https://phabricator.haskell.org/D5134
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Long ago, the stable name table and stable pointer tables were one.
Now, they are separate, and have significantly different
implementations. I believe the time has come to finish the split
that began in #7674.
* Divide `rts/Stable` into `rts/StableName` and `rts/StablePtr`.
* Give each table its own mutex.
* Add FFI functions `hs_lock_stable_ptr_table` and
`hs_unlock_stable_ptr_table` and document them.
These are intended to replace the previously undocumented
`hs_lock_stable_tables` and `hs_lock_stable_tables`,
which are now documented as deprecated synonyms.
* Make `eqStableName#` use pointer equality instead of unnecessarily
comparing stable name table indices.
Reviewers: simonmar, bgamari, erikd
Reviewed By: bgamari
Subscribers: rwbarton, carter
GHC Trac Issues: #15555
Differential Revision: https://phabricator.haskell.org/D5084
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Summary: These can be treated similarly to MUT_ARRY_PTRS. Fixes #15529.
Reviewers: erikd, simonmar
Reviewed By: simonmar
Subscribers: RyanGlScott, rwbarton, carter
GHC Trac Issues: #15529
Differential Revision: https://phabricator.haskell.org/D5075
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
While investigating #15529, I noticed that the `barf`ed
error message in `pop()` doesn't print out the closure type that
causes it to crash. Let's do so.
Reviewers: bgamari, erikd, simonmar
Reviewed By: bgamari
Subscribers: rwbarton, carter
GHC Trac Issues: #15529
Differential Revision: https://phabricator.haskell.org/D5072
|
|
|
|
|
|
|
|
|
|
|
|
| |
Test Plan: Validate
Reviewers: erikd, simonmar
Reviewed By: simonmar
Subscribers: rwbarton, thomie, carter
Differential Revision: https://phabricator.haskell.org/D4798
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
SMALL_MUT_ARR_PTRS_FROZEN0 -> SMALL_MUT_ARR_PTRS_FROZEN_DIRTY
SMALL_MUT_ARR_PTRS_FROZEN -> SMALL_MUT_ARR_PTRS_FROZEN_CLEAN
MUT_ARR_PTRS_FROZEN0 -> MUT_ARR_PTRS_FROZEN_DIRTY
MUT_ARR_PTRS_FROZEN -> MUT_ARR_PTRS_FROZEN_CLEAN
Naming is now consistent with other CLEAR/DIRTY objects (MVAR, MUT_VAR,
MUT_ARR_PTRS).
(alternatively we could rename MVAR_DIRTY/MVAR_CLEAN etc. to MVAR0/MVAR)
Removed a few comments in Scav.c about FROZEN0 being on the mut_list
because it's now clear from the closure type.
Reviewers: bgamari, simonmar, erikd
Reviewed By: simonmar
Subscribers: rwbarton, thomie, carter
Differential Revision: https://phabricator.haskell.org/D4784
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Recent changes to SRTs (D4632, D4637) also required changes to
RetainerProfile.c. This should hopefully get things working again.
Test Plan: validate with profiling turned on
Reviewers: bgamari, osa1, tdammers, erikd
Subscribers: rwbarton, thomie, carter
Differential Revision: https://phabricator.haskell.org/D4707
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Summary:
The idea here is to save a little code size and some work in the GC,
by collapsing FUN_STATIC closures and their SRTs.
This is (4) in a series; see D4632 for more details.
There's a tradeoff here: more complexity in the compiler in exchange
for a modest code size reduction (probably around 0.5%).
Results:
* GHC binary itself (statically linked) is 1% smaller
* -0.2% binary sizes in nofib (-0.5% module sizes)
Full nofib results comparing D4634 with this: P177 (ignore runtimes,
these aren't stable on my laptop)
Test Plan: validate, nofib
Reviewers: bgamari, niteria, simonpj, erikd
Subscribers: thomie, carter
Differential Revision: https://phabricator.haskell.org/D4637
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
push() considers BLOCKING_QUEUES to be an invalid closure type which
should never be present on the stack. However, retainClosure made no
accomodation for this and ended up pushing such a closure. This lead
to #14947.
Test Plan: Validate
Reviewers: simonmar, erikd
Reviewed By: simonmar
Subscribers: thomie, carter, RyanGlScott
GHC Trac Issues: #14947
Differential Revision: https://phabricator.haskell.org/D4538
|
|
|
|
|
|
|
|
|
|
| |
Reviewers: bgamari, simonmar, erikd
Reviewed By: bgamari, simonmar
Subscribers: thomie, carter
Differential Revision: https://phabricator.haskell.org/D4539
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
While investigating #14947, I noticed that the `barf`ed
error message in `push()` doesn't print out the closure type that
causes it to crash. Let's do so.
Reviewers: bgamari, erikd, simonmar
Reviewed By: bgamari
Subscribers: alexbiehl, rwbarton, thomie, carter
Differential Revision: https://phabricator.haskell.org/D4525
|
|
|
|
|
|
|
|
|
|
|
|
| |
Reviewers: bgamari, erikd, simonmar
Reviewed By: simonmar
Subscribers: rwbarton, thomie, carter
GHC Trac Issues: #11777
Differential Revision: https://phabricator.haskell.org/D4427
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently runtime keeps hold to 4*used_memory. This includes, in
particular, nursery, which can be quite large on multiprocessor
machines: 16 CPUs x 64Mb each is 1GB. Multiplying it by 4 means whatever
actual memory usage is, runtime will never release memory under 4GB, and
this is quite excessive for processes which only need a lot of memory
shortly (think building data structures from large files).
This diff makes multiplier to apply only to GC-managed memory, leaving
all "static" allocations alone.
Test Plan: make test TEST="T14702"
Reviewers: bgamari, erikd, simonmar
Reviewed By: simonmar
Subscribers: rwbarton, thomie, carter
GHC Trac Issues: #14702
Differential Revision: https://phabricator.haskell.org/D4338
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
orzo in `#ghc` reported seeing a crash due to the retainer profiler encountering
a BLOCKING_QUEUE closure, which isRetainer didn't know about. I performed an
audit to make sure that all of the valid closure types were listed; they
weren't. This is my guess of how they should appear.
Test Plan: Validate
Reviewers: simonmar, austin, erikd
Reviewed By: simonmar
Subscribers: rwbarton, thomie
GHC Trac Issues: #14235
Differential Revision: https://phabricator.haskell.org/D3967
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
gcc complains about this while building with Hadrian,
```
rts/RetainerProfile.c: In function ‘computeRetainerSet’:
rts/RetainerProfile.c:1758:18: error:
error: variable ‘rtl’ set but not used
[-Werror=unused-but-set-variable]
RetainerSet *rtl;
^~~
|
1758 | RetainerSet *rtl;
| ^
```
Reviewers: austin, erikd, simonmar, Phyx
Reviewed By: Phyx
Subscribers: Phyx, rwbarton, thomie
Differential Revision: https://phabricator.haskell.org/D3801
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These were found while using Hadrian, which apparently uses slightly
stricter warning flags than the make-based build system.
Test Plan: Validate
Reviewers: austin, erikd, simonmar
Reviewed By: erikd
Subscribers: rwbarton, thomie
Differential Revision: https://phabricator.haskell.org/D3679
|
|
|
|
| |
Our new CPP linter enforces this.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Test Plan: Validate on lots of platforms
Reviewers: erikd, simonmar, austin
Reviewed By: erikd, simonmar
Subscribers: michalt, thomie
Differential Revision: https://phabricator.haskell.org/D2699
|