puffin icon indicating copy to clipboard operation
puffin copied to clipboard

Fix saving as `.puffin` file

Open emilk opened this issue 4 months ago • 8 comments

  • Fixes https://github.com/EmbarkStudios/puffin/issues/248 ?
  • Probably broke in https://github.com/EmbarkStudios/puffin/pull/169

emilk avatar Aug 05 '25 14:08 emilk

I spoke too soon.

gwen-lg avatar Aug 05 '25 14:08 gwen-lg

I have tested with commit 15e131c2f92a8f3f0941167249210eb3754a7ab1 (which contain change from https://github.com/EmbarkStudios/puffin/pull/169) and main (with revert update to egui v0.31 who failed to build for me).

And saving a capture in .puffin file seems to work for me. @emilk : What problem did you encounter?

gwen-lg avatar Aug 05 '25 15:08 gwen-lg

The scope info (names etc) wasn't being saved to the .puffin file, so a fresh puffin_viewer would show no scopes.

This silent error will be less silent with https://github.com/EmbarkStudios/puffin/pull/245

emilk avatar Aug 05 '25 16:08 emilk

I don't have this problem. And I don't understand why you have this problem. In FrameData.write_into, if scope_collection parameter is None scope info are written from self.scope_delta. Unless in your case, unlike in my case, scope_delta is cleared ?

gwen-lg avatar Aug 05 '25 16:08 gwen-lg

My repro: connect puffin_viewer to a server with --url 127.0.0.1:8585, hit save, then try to load the saved ~.rrd~ .puffin in a new puffin_viewer process.

Didn't work before this PR, but works with this PR.

emilk avatar Aug 05 '25 17:08 emilk

.rrd ? I use Save as entry in puffin_viewer, and it save a .puffin file. And after close and restart puffin_viewer, when I open the previously saved file, I have all the scope information. For test, I used a very simple app, with very few scope, I will tests with more complicated profiling.

gwen-lg avatar Aug 05 '25 17:08 gwen-lg

Sorry, I meant .puffin.

I tried my repro again, and now it works on main O.o. This is very weird.

Before I ended up with this broken file: broken.puffin.zip

Maybe something else is wrong.

To be honest, https://github.com/EmbarkStudios/puffin/pull/169 added so much complexity that I am considering reverting it

  • For instance: https://github.com/EmbarkStudios/puffin/issues/200

emilk avatar Aug 05 '25 17:08 emilk

Reducing bandwidth ans cpu use by send Stream scope information only once seems to be a good idea, but I haven't look how it is implemented. I have planned to take a look at your file broken.puffin to investigate. And I can take a look at the code of Streaming scope info only once, and see if I see something or see possible complexity reduction. If possible wait before revert it.

gwen-lg avatar Aug 07 '25 21:08 gwen-lg