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

improved save speed of session file #443

Closed
wants to merge 4 commits into from

Conversation

maruncz
Copy link

@maruncz maruncz commented Nov 16, 2021

main issue was in sr_sessoin_append, where it was opening and closing that zipfile every time, i moved this operation outside this function. It is not pretty solution, but it is fast solution.
I also had to modify soresession.cpp because it would abort saving when it left loop. i added dummy bytes to write and marked them written after closing zipfile

@christianeisendle
Copy link

Indeed, i was working on exact same issue yesterday and did came up with already identical solution :-) Haven't seen this PR only now. I didn't push my work yet because i would like to have it working for windows (see #459 and #441)
On top i did:

  • Added zip_register_progress_callback_with_state to synchronize the progress bar with zip_close operation
  • Add zip_set_file_compression in sr_session_append to set compression to store only. With that saving is extremely fast, but it produces huge archives (which could be manually zipped if needed)

The main issue i see is how session are saved in general: While the UI handles data efficiently and only store changes in RAM, saving the session is done by storing plain/raw samples, So in other words: If you have a trace which takes 10sec at 100MHz but has only one 0 -> 1 transition on one channel, then this takes zero RAM in the UI (only the value change is stored). However, once you save the session, it deflates the dump and stores every single sample on the hard disk. If this logic would be changes to only store changes (e.g. VCD format) then this would save significant disk space and would also speed up the session saving. But this would be a bit more effort and currently i'm a bit frustrated because i anyhow can't build DSview under Windows :-)

@maruncz
Copy link
Author

maruncz commented Jan 4, 2022

Hi, I think that proper solution would be merging upstreab libsigrok. I assume that they already worked on this.
Regarding compression, i also experimented with STORE but i found that i'm getting HDD bottleneck and filesizes were impractical.

@maruncz
Copy link
Author

maruncz commented Jan 4, 2022

Could you share your version with zip_register_progress_callback_with_state? I would like to add it to my build.

@christianeisendle
Copy link

Could you share your version with zip_register_progress_callback_with_state? I would like to add it to my build.

Here you go:
christianeisendle@896d6b4

@christianeisendle
Copy link

Hi, I think that proper solution would be merging upstreab libsigrok. I assume that they already worked on this.

Indeed they did. There is now an output module srzip.c as part of libsigrok. sigrok-cli uses it to real-time stream into the zip file by opening it only once. Still, they dump the complete data, e.g. all samples and not just the value changes. (i think their intent is that zip anyhow achieves a high compression rate with lots of redundancy, i.e. long series of 0's or 1's)

@maruncz
Copy link
Author

maruncz commented Mar 2, 2022

hi @christianeisendle
i just found that both of us have memory leak use-after-free in our code. check my last commit.
sr_session_append does not copy content of buf pointer, so if needs_malloc branch is taken buf is freed, but it is accesed when closing file in zip_close

@dreamsource-tai
Copy link
Collaborator

Hi! The new version have implemented this function.

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.

None yet

3 participants