JuliaPerf / PProf.jl

Export Julia profiles to the pprof format
MIT License
155 stars 17 forks source link

CompatHelper: bump compat for ProtoBuf to 0.11, (keep existing compat) #42

Closed github-actions[bot] closed 2 years ago

github-actions[bot] commented 2 years ago

This pull request changes the compat entry for the ProtoBuf package from 0.7, 0.8 to 0.7, 0.8, 0.11. This keeps the compat entries for earlier versions.

Note: I have not tested your package with this new compat entry. It is your responsibility to make sure that your package tests pass before you merge this pull request.

NHDaly commented 2 years ago

did the tests run on this PR, @vchuravy?

I saw that the 0.9 and 0.10 upgrade PRs had broken builds due to a change in the proto package: https://github.com/JuliaPerf/PProf.jl/pull/34#issuecomment-714170923 https://github.com/JuliaPerf/PProf.jl/pull/35#issuecomment-999628443

I can't tell but it looks like maybe tests didn't run here?

NHDaly commented 2 years ago

maybe we need to double check our CI config... 🤔

NHDaly commented 2 years ago

Mmm yeah, i think this broke master: https://github.com/JuliaPerf/PProf.jl/runs/4628232421?check_suite_focus=true

I'll revert it.

NHDaly commented 2 years ago

maybe we need to double check our CI config... 🤔

Huh, i don't see anything obvious in the CI config why it didn't run tests.... and the rollback PR has tests running: https://github.com/JuliaPerf/PProf.jl/pull/43 https://github.com/JuliaPerf/PProf.jl/runs/4638518767?check_suite_focus=true

NHDaly commented 2 years ago

@vchuravy: @dewilson (from RAI) would like us to add support for proto 0.11, because she wants us to benchmark an internal implementation of data persistence with the ProtoBuf.jl package.

But apparently she's discovered that the later versions (@dewilson: is it starting in 0.9?) of ProtoBuf.jl switch from generating static struct files like we saw to instead generating fully generic proto definitions that use Dicts internally.

We are concerned about the perf characteristics of allocating one Dict per proto message.. So we're a bit sketched about upgrading, and still trying to understand more at the moment.