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

Support the newly added par_balanced_copied field. #47

Merged
merged 5 commits into from
May 15, 2019

Conversation

sighingnow
Copy link
Contributor

Add a fixed size parser that can handle the newer version of GCStatsGHC Event.

Fixes #31 and #41.

@maoe
Copy link
Member

maoe commented May 14, 2019

Could you add a small eventlog which contains the new field to the test suite?

@sighingnow
Copy link
Contributor Author

Done.

The hello-ghc-x.x.x.eventlog demonstrates the difference of GCStatsGHC generated by different version of GHC. The existing stop.hT.eventlog also have 58-bytes GCStatsGHC events and this PR have updated the stop.hT.eventlog.reference as well.

Copy link
Member

@maoe maoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. LGTM besides the minor requests below.

parBalancedCopied <- return Nothing
return GCStatsGHC{ gen = fromIntegral gen
, parNThreads = fromIntegral parNThreads
, ..}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to set parBalancedCopied here rather than using return Nothing above:

Suggested change
, ..}
, parBalancedCopied = Nothing
, ..}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks for the suggestion.

parNThreads <- get :: Get Word32
parMaxCopied <- get :: Get Word64
parTotCopied <- get :: Get Word64
parBalancedCopied <- fmap Just (get :: Get Word64)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here:

Suggested change
parBalancedCopied <- fmap Just (get :: Get Word64)
parBalancedCopied <- get :: Get Word64
return GCStatsGHC{ gen = fromIntegral gen
, parNThreads = fromIntegral parNThreads
, parBalancedCopied = Just parBalancedCopied
, .. }

@maoe
Copy link
Member

maoe commented May 15, 2019

LGTM, thanks!

@maoe maoe merged commit b784965 into haskell:master May 15, 2019
@sighingnow sighingnow deleted the fix-gcstatsghc branch May 15, 2019 05:36
@maoe
Copy link
Member

maoe commented May 15, 2019

@sighingnow
Copy link
Contributor Author

@maoe Thanks!

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

Successfully merging this pull request may close these issues.

Support new field 'par_balanced_copied_bytes' of EVENT_GC_STATS_GHC
2 participants