Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ProfSampleCostCentre in the eventlog file carries a CapNo instead of a Capset #84

Open
nfrisby opened this issue May 6, 2022 · 2 comments

Comments

@nfrisby
Copy link

nfrisby commented May 6, 2022

I think the first field of ProfSampleCostCentre should be profCap :: !CapNo instead of profCapset :: !Capset.

| ProfSampleCostCentre
{ profCapset :: !Capset
, profTicks :: !Word64
, profStackDepth :: !Word8
, profCcsStack :: !(VU.Vector Word32)
}

timeProfParsers :: [EventParser EventInfo]
timeProfParsers = [
FixedSizeParser EVENT_PROF_BEGIN 8 $ do
profTickInterval <- get
return $! ProfBegin{..}
, VariableSizeParser EVENT_PROF_SAMPLE_COST_CENTRE $ do
payloadLen <- get :: Get Word16
profCapset <- get
profTicks <- get
profStackDepth <- get
profCcsStack <- VU.replicateM (fromIntegral profStackDepth) get
assert
((fromIntegral payloadLen :: Int) == sum
[ 4
, 8 -- ticks
, 1 -- stack depth
, fromIntegral profStackDepth * 4
])
(return ())
return $! ProfSampleCostCentre {..} ]

https://github.com/ghc/ghc/blob/73b22ff196160036ac10b762bf3a363fa8a451ad/rts/eventlog/EventLog.c#L1317-L1340 has

void postProfSampleCostCentre(Capability *cap,
                              CostCentreStack *stack,
                              StgWord64 tick)
{
    /* SNIP */
    postEventHeader(&eventBuf, EVENT_PROF_SAMPLE_COST_CENTRE);
    postPayloadSize(&eventBuf, len);
    postWord32(&eventBuf, cap->no);                                                /* THIS IS THE NOTABLE LINE */
    postWord64(&eventBuf, tick);
    /* SNIP */
}
  • This seems to be an inconsistency between ghc master and ghc-events master -- I haven't done the due diligence to check wheter ghc master has changed this recently, eg.

  • Or perhaps there's an implicit assumption that CapNo i is the sole member of Capset i?

So either there's a mismatch that should be fixed or I think it's deserving of a comment on profCapset. Thanks!

@maoe
Copy link
Member

maoe commented May 11, 2022

Yeah that indeed looks like a mismatch that should be fixed.

@mpickering Apparently you wrote both implementations in 9f00d38 and in ghc/ghc@17987a4. Was this intentional?

@mpickering
Copy link
Contributor

I think it's probably a mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants