summaryrefslogtreecommitdiff
path: root/rts/RetainerProfile.c
Commit message (Collapse)AuthorAgeFilesLines
* Fix typosBrian Wignall2019-11-231-1/+1
|
* rts: RetainerProfile: Explain retainVisitClosure return valuesDaniel Gröber2019-09-221-3/+3
| | | | [ci skip]
* rts: Split heap traversal from retainer profilerDaniel Gröber2019-09-221-1353/+0
| | | | | This finally moves the newly generalised heap traversal code from the retainer profiler into it's own file.
* rts: RetainerProfile.c: Minimize #includesDaniel Gröber2019-09-221-8/+1
| | | | | A lot of these includes are presumably leftovers from when the retainer profiler still did it's own heap profiling.
* rts: RetainerProfile.c: Re-enable and fix warningsDaniel Gröber2019-09-221-9/+11
| | | | | | 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.. :)
* rts: retainer: Improve Note [Profiling heap traversal visited bit]Daniel Gröber2019-09-221-20/+20
|
* rts: retainer: Make visit callback easier to implementDaniel Gröber2019-09-221-11/+16
| | | | | | | | | 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.
* rts: retainer: Move mut_list reset to generic traversal codeDaniel Gröber2019-09-221-40/+33
|
* rts: retainer: Remove traverse-stack chunk supportDaniel Gröber2019-09-221-38/+7
| | | | | | | | | | | | | | | | | | 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")
* rts: retainer: Move actual 'flip' bit flip to generic traversal codeDaniel Gröber2019-09-221-3/+5
|
* rts: retainer: Update obsolete docs for traverseMaybeInitClosureDataDaniel Gröber2019-09-221-14/+6
|
* rts: retainer: Abstract maxStackSize for generic traversalDaniel Gröber2019-09-221-3/+10
|
* rts: retainer: Move heap traversal declarations to new headerDaniel Gröber2019-09-221-88/+7
|
* rts: retainer: Use global STATIC_INLINE macroDaniel Gröber2019-09-221-25/+18
| | | | | STATIC_INLINE already does what the code wanted here, no need to duplicate the functionality here.
* rts: retainer: Remove outdated invariants on traversePushStackDaniel Gröber2019-09-221-8/+0
| | | | | | 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*'.
* rts: retainer: Cleanup comments and strings for traversal extractionDaniel Gröber2019-09-221-95/+137
| | | | | A lot of comments and strings are still talking about old names, fix that.
* rts: retainer: Reduce DEBUG_RETAINER ifdef noiseDaniel Gröber2019-09-221-54/+37
| | | | | | | | | | 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.
* rts: retainer: Rename heap traversal functions for extractionDaniel Gröber2019-09-221-100/+99
| | | | | This gets all remaining functions in-line with the new 'traverse' prefix and module name.
* rts: retainer: Remove obsolete debug codeDaniel Gröber2019-09-221-330/+1
| | | | | | | 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.
* rts: RetainerSet: Remove obsolete fist/second-approach choiceDaniel Gröber2019-09-221-19/+0
| | | | | | | | | | | | | 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 :)
* rts: retainer: simplify pop() control flowDaniel Gröber2019-09-221-33/+38
| | | | | | | | | | | | | | | | | | 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.
* rts: retainer: Pull retainer specific code into a callbackDaniel Gröber2019-09-221-125/+131
| | | | | | | 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.
* rts: Generalise profiling heap traversal flip bit handlingDaniel Gröber2019-09-221-20/+41
| | | | | | | 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.
* rts: retainer: Fix comment typo s/keeps/keep/Daniel Gröber2019-09-221-1/+1
|
* rts: retainer: Generalise per-stackElement dataDaniel Gröber2019-09-221-63/+75
| | | | | | | | | | | | | | | 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.
* rts: retainer: Move info.next.parent to stackElementDaniel Gröber2019-09-221-6/+5
| | | | | I don't see a point in having this live in 'info', just seems to make the code more complicated.
* rts: retainer: Turn global traversal state into a structDaniel Gröber2019-09-221-161/+170
| | | | | | | | | | 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.
* rts: retainer: Remove cStackSize debug counterDaniel Gröber2019-09-221-22/+2
| | | | | This can only ever be one since 5f1d949ab9 ("Remove explicit recursion in retainer profiling"), so it's pointless.
* rts: Fix RetainerProfile early return with TREC_CHUNKDaniel Gröber2019-06-091-1/+1
| | | | | | | | 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.
* RetainerProfiler: Update retainer profiler debuggingAlexander Vershilov2018-12-121-125/+85
| | | | | | | | | | | | | 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
* Remove explicit recursion in retainer profiling (fixes #14758)Alexander Vershilov2018-12-051-77/+146
| | | | | | | | | | | | | | | | | | | | | | | 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
* Various RTS bug fixes:Ömer Sinan Ağacan2018-09-071-2/+1
| | | | | | | | | | | | | | | | | - 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
* Finish stable splitDavid Feuer2018-08-291-2/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* rts: Handle SMALL_MUT_ARR_PTRS in retainer profilterBen Gamari2018-08-281-0/+4
| | | | | | | | | | | | | | 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
* rts/RetainerProfile: Dump closure type if pop() failsRyan Scott2018-08-211-1/+1
| | | | | | | | | | | | | | | | 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
* rts: Fix reference to srt_bitmap in ASSERT in RetainerProfileBen Gamari2018-06-071-1/+1
| | | | | | | | | | | | Test Plan: Validate Reviewers: erikd, simonmar Reviewed By: simonmar Subscribers: rwbarton, thomie, carter Differential Revision: https://phabricator.haskell.org/D4798
* Rename some mutable closure types for consistencyÖmer Sinan Ağacan2018-06-051-10/+10
| | | | | | | | | | | | | | | | | | | | | | | 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
* Fix retainer profiling after SRT overhaulSimon Marlow2018-05-191-147/+28
| | | | | | | | | | | | | 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
* Merge FUN_STATIC closure with its SRTSimon Marlow2018-05-161-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | 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
* rts/RetainerProfile: Handle BLOCKING_QUEUESBen Gamari2018-04-101-1/+10
| | | | | | | | | | | | | | | | | | | 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
* Remove unused bdescr flag BF_FREEÖmer Sinan Ağacan2018-04-051-1/+1
| | | | | | | | | | Reviewers: bgamari, simonmar, erikd Reviewed By: bgamari, simonmar Subscribers: thomie, carter Differential Revision: https://phabricator.haskell.org/D4539
* rts/RetainerProfile: Dump closure type if push() failsRyan Scott2018-03-251-1/+1
| | | | | | | | | | | | | | 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
* RTS: Remove unused retainer schemesSimon Jakobi2018-02-251-9/+0
| | | | | | | | | | | | Reviewers: bgamari, erikd, simonmar Reviewed By: simonmar Subscribers: rwbarton, thomie, carter GHC Trac Issues: #11777 Differential Revision: https://phabricator.haskell.org/D4427
* Make RTS keep less memory (fixes #14702)Andrey Sverdlichenko2018-01-311-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | 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
* rts/RetainerProfile: Adding missing closure types to isRetainerBen Gamari2017-09-191-0/+23
| | | | | | | | | | | | | | | | | | | 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
* rts: Fix "variable set but not used" warningBen Gamari2017-08-011-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | 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
* rts/RetainerProfile: Const-correctness fixesBen Gamari2017-06-291-9/+9
| | | | | | | | | | | | | | | 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
* Prefer #if defined to #ifdefBen Gamari2017-04-281-41/+41
| | | | Our new CPP linter enforces this.
* Typos [ci skip]Gabor Greif2017-02-151-1/+1
|
* Use C99's boolBen Gamari2016-11-291-14/+14
| | | | | | | | | | | | 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