search

haskell / ghc-events

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

add CPP for ghc-8.4 compatibility, bump version to 0.7.2 #36

Closed elaforge closed 6 years ago

elaforge commented 6 years ago

This lets ghc-events build on ghc 8.4.

This tried to compile but crashed because of the base 4.* constraint. Aren't you supposed to use <= known version for base?

maoe commented 6 years ago

Could you add GHC 8.4.1 to the Travis build matrix in .travis.yml to see if it actually compiles?

This tried to compile but crashed because of the base 4.* constraint. Aren't you supposed to use <= known version for base?

Agreed. That line was written back in 2009. Could you update it as well? I think >= 4 && < 4.12 should be fine.

maoe commented 6 years ago

Also it's not a big deal but you don't need to bump the version number nor add a changelog section with the version number. No one knows when the release date will be and the next version can be a major version bump due to other changes. The maintainers do it when s/he is releasing. Maybe we should prepare a CONTRIBUTING.md..

elaforge commented 6 years ago

Ok, let me try the travis and cabal changes. One moment.

I bumped the version number because presumably we want to do a release right away. Otherwise any project that depends on this can't use ghc 8.4.

elaforge commented 6 years ago

Ok, updated and looks like travis has picked it up. .travis.yml has a bunch of changes, but since it's generated from a script, I'll just trust that it knows what it's doing.

elaforge commented 6 years ago

Older ghc versions failed on the bytestring dependency. I don't see why changing the base restriction and adding a new ghc should make old versions fail on bytestring, but maybe it has to do the travis generator changing.

It looks like it supplies bytestring 0.10.0.0, and ghc-events wants >= 0.10.4, but in that case it shouldn't have worked before either.

elaforge commented 6 years ago

Ok I think I see. According to https://ghc.haskell.org/trac/ghc/wiki/Commentary/Libraries/VersionHistory, ghc <=7.6.3 ships with bytestring 0.10.0.2, so ghc-events hasn't supported those versions for as long as the bytestring constraint hsa been present, which looks like 7a8b509aa89b9ca16e09884d78d32abf1e3360ff, by you, on Feb 10 07:19:40 2017.

Likely the reason why the breakage wasn't caught is that the check against installed libraries is new, and is indeed due to make_travis_yml_2.hs being updated and adding a new check. Previously I think it would just install a newer bytestring and pass.

I can either turn off that check, or I can remove ghc <=7.6.3 from tested-with, or you could do some #ifdef to have 7a8b509aa89b9ca16e09884d78d32abf1e3360ff only apply when a newer bytestring is present. Turning of the check is a bit sketchy, bytestring is a boot lib and it's hard to say you support a ghc version if it requires overriding a boot lib. But that would be maintaining status quo. Removing the old versions from support seems ok, I think there are not that many users, and I hope if you care about performance you are upgrading ghc. The ifdef stuff seems like too much hassle.

For the short term I'll turn off the INSTALLED check just to make it pass, since the breakage doesn't actually have anything to do with my pull request.

maoe commented 6 years ago

I don't have a strong opinion here and am fine either of disabling the installed check or dropping the support for ghc <= 7.6.3. We certainly don't need to employ #ifdef to support that old compilers.

@Mikolaj @kvelicka What do you think?

Mikolaj commented 6 years ago

I think very few people use 7.6.3 these days and if they do, they don't need 8.4 compatibility, so they can stay at old ghc-events versions, so I'd drop 7.6.3. BTW, great job @elaforge.

kvelicka commented 6 years ago

+1 for removing the constraint, same reasoning as Mikolaj.

maoe commented 6 years ago

Okay. Let's drop the support for ghc <= 7.6.3 then. @elaforge Could you please update the PR?

elaforge commented 6 years ago

Updated and travis is happy now.

Can I get someone to do a hackage release after merging? My main goal is to get my project compiling with 8.4. I could do the release but I probably don't have upload privileges.

maoe commented 6 years ago

Thanks! I can merge this and upload a new version in a few hours.

maoe commented 6 years ago

Looks perfect! Thank you again for your work.

maoe commented 6 years ago

Uploaded https://hackage.haskell.org/package/ghc-events-0.7.2

maoe commented 6 years ago

@elaforge May I ask how you use this library in your project if you don't mind? I'm curious about usage patterns of the library beside ThreadScope and ghc-events-analyze.

elaforge commented 6 years ago

I have a converter that writes JSON read by chrome://trace. This is like threadscope, but has more features and is already installed wherever you happen to have chrome. Currently I use it for my own project, but eventually I want to generalize and put on hackage.

maoe commented 6 years ago

That sounds cool. How is the memory usage of the event viewer? Can you look into a huge eventlog file without a memory hog?

I'm meaning to implement some memory-efficient indexing in ghc-events and use it from ThreadScope to fix the memory hog.

elaforge commented 6 years ago

I haven't tried it with large traces yet. I'm guessing chrome can handle pretty big ones, because it's primary use is debugging performance issues in web pages down to low level rendering, and those generate a lot of events.

I'm pretty sure it still loads the whole file into memory at once though, so if by huge you mean won't fit in RAM, than it probably can't handle that.

Still, a streaming interface to ghc-events would be nice, so I could convert a file without having to load it all into memory. I've been using streaming on hackage, and it's pretty straightforward.

maoe commented 6 years ago

A quick google search turned up https://github.com/golang/go/issues/11520, which is unresolved yet.

Still, a streaming interface to ghc-events would be nice, so I could convert a file without having to load it all into memory.

I guess this is doable today, isn't it? Thanks to @kvelicka, we have a streaming interface. I have no idea what the trace format in Chrome looks like but if you don't need to sortEvents, you should be able to convert an eventlog in constant space.

Anyway, please let me know when you publish your work. I have huge eventlogs around so I can test.

elaforge commented 6 years ago

Ah so there is. It should be easy to plug that interface into a stream. Chrome has some restrictions about sortedness, but also some freedom, it depends on the event type. In fact it might be the same semi-sorted that GHC produces naturally... something like sorted within a thread, but no guarantee across threads? I thought I read somewhere it uses per-thread buffers that causes that.

For large eventlogs, I planned to have a heuristic to find interesting ranges, such as ones that exceeded a latency threshold, and emit those separately. That would avoid the need to load the whole thing in one go, and reduce the need to scroll and zoom and try to find interesting things by eye.

I will announce when I have something usable.