search

haskell / ghc-events

Library and tool for parsing .eventlog files from GHC
http://www.haskell.org/haskellwiki/ThreadScope
Other
33 stars 33 forks source link

Drop support for GHC < 8 #89

Closed TeofilC closed 2 years ago

TeofilC commented 2 years ago

This MR removes parsers and workarounds specific to GHC < 8. I've also removed some test cases for older eventlogs.

Resolves #88

Mikolaj commented 2 years ago

@bgamari, @mpickering, @AndreasPK: since you are the only producers of ghc-events data and you keep a copy of the file .eventlog format in GHC --- do you see any obstacle to dropping compatibility with 5 years old GHCs? How could this affect your copy of the code?

maoe commented 2 years ago

I don't use GHC < 8.0 any more so I'm personally fine with this change.

I feel like dropping support for those old versions would help simplify the library and unblock some of the open issues.

I'm interested in this remark. What issue does this change unblock specifically?

TeofilC commented 2 years ago

I think I had this comment in mind https://github.com/haskell/ghc-events/issues/14#issuecomment-417917789

AndreasPK commented 2 years ago

@bgamari, @mpickering, @AndreasPK: since you are the only producers of ghc-events data and you keep a copy of the file .eventlog format in GHC --- do you see any obstacle to dropping compatibility with 5 years old GHCs? How could this affect your copy of the code?

I don't think there should be issues for ghc itself in that regard.

Mikolaj commented 2 years ago

I don't think there should be issues for ghc itself in that regard.

Thank you. In that case, even code cleanup could be reason enough. After all, maintenance takes work and hairy code makes it harder.

maoe commented 2 years ago

I think I had this comment in mind #14 (comment)

I see. If that issue could be fixed by simply dropping the old GHC support as suggested, that would be great.

I'm +1 to this change.

I'm going to ping @simonmar as he said

ghc-events was specifically designed to be forwards and backwards compatible as far as possible.

in #27 in the past. Do you mind if we drop support for GHC < 8 now?

maoe commented 2 years ago

@TeofilC Have you checked if this PR fixes #14?

TeofilC commented 2 years ago

It doesn't fix #14 unfortunately. I tried re-enabling the test suites and they still fail. I think the remaining issue is due to events being reordered because of how EventBlock is handled.

maoe commented 2 years ago

It doesn't fix https://github.com/haskell/ghc-events/issues/14 unfortunately.

That's unfortunate.

That said, it'd still be nice to get rid of some old code along with the relevant test data for simplicity. Also there have been no objections since this was submitted almost a month ago. I think we can go ahead.