
4. In `unitFV` I see {{{ unitFV :: Id -> FV unitFV var fv_cand in_scope acc@(have, haveSet) | var `elemVarSet` in_scope = acc | var `elemVarSet` haveSet = acc | fv_cand var = (var:have, extendVarSet haveSet var) | otherwise = acc }}} Notice that ''before inserting we test for membership''. Could that
#14880: GHC panic: updateRole -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: goldfire Type: bug | Status: new Priority: normal | Milestone: Component: Compiler (Type | Version: 8.2.2 checker) | Resolution: | Keywords: TypeInType Operating System: Unknown/Multiple | Architecture: Type of failure: Compile-time | Unknown/Multiple crash or panic | Test Case: Blocked By: | Blocking: Related Tickets: #15076 | Differential Rev(s): Phab:D4769 Wiki Page: | -------------------------------------+------------------------------------- Comment (by tdammers): Replying to [comment:59 simonpj]: possibly be a win? If so, we can use it in the `VarSet` version. I believe that the "test before insert" thing is needed because we're accumulating a set (`haveSet`) and a list (`have`) in parallel, instead of just the set, so in order to keep the two in sync, we cannot simply insert unconditionally, as that would produce duplicates in the list. I don't think it can possibly produce a performance advantage beyond skipping the list cons, *unless* we are in a special situation where: - `in_scope` is much smaller than `haveSet` (such that testing membership of `in_scope` is significantly faster than testing `haveSet` membership); AND: - `in_scope` is likely to match new entries (because when it doesn't match, the extra test just eats up clock cycles without gaining anything) One interesting thing might be to invert the `fv_cand` condition and change the ordering, so that the `fv_cand` test is done before the set membership tests, something like: {{{#!haskell unitFV :: Id -> FV unitFV var fv_cand in_scope acc@(have, haveSet) | not (fv_cand var) = acc | var `elemVarSet` in_scope = acc | var `elemVarSet` haveSet = acc | otherwise = (var:have, extendVarSet haveSet var) }}} But that won't change the performance of `VarSet` the slightest. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14880#comment:63 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler