-
Notifications
You must be signed in to change notification settings - Fork 237
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
Progress reporting introduces large performance hit #87
Comments
Not incurring a big performance hit to report progress is a definite requirement (for me at least). I do not think you even need to have an Have you experimented with any changes to reduce the performance hit yet? I am not that familiar with the indicators implementation, so I would be glad to work with you on this. As I said, I cannot use a progress reporting mechanism that is going to incur a significant performance penalty or block a thread in any way. So I either need to do something to indicators or keep looking for a new solution. |
As my algorithm was fast enough that I could just remove the progress usage in my codebase, this became low priority for me and I haven't tested anything, preferring to wait to get some sort of feedback on the interface design. I like the chrono::duration idea. It simplifies the interface in an intuitive way. One thing I'm noting now to help me remember during implementation, the progress output should always get printed for the final tick/set_progress/mark_as_completed call. I'm under a deadline for the next few weeks, so I probably won't get to look at this seriously for a little bit longer. If you wanted to take a stab at it, the code is pretty self explanatory. I'd probably start by adding a check to |
I was running into the same issue. I have another idea to reduce the printing load that is not time based. Which should be faster since getting the time is a system call. When the max, min progress and granularity is set the indicator calculates the amount of progress that has to be made until it prints again. |
Weirdly, this issue just popped up for me again in my code within the last week. Guess it's a good time for people running long running tasks. From a UX perspective, I personally think I'm more interested in having the bar update the eta after it reaches a specified printing interval rather than waiting for it to report on a percentage. Particularly with long running tasks, the user always thinks it's hung. Is it hung at 1% or just taking a long time to get to 2%? Updating something in the progress bar helps that. I haven't profiled it, but I also think getting the system time is probably marginal compared to the current time spent flushing the buffer. Anyway, I also see the merit in your approach. Not sure how compatible the two ideas are given the current code, but seems at least possible. |
In my case, I got the performance back by calling set_progress only when I needed to:
It would be nice if the framework could have similar logic internally. |
I recently discovered that one of my algorithms which I thought was long running (~7min), was actually quite fast when I disabled the progress reporting (15s!!!). This doesn't particularly surprise me since the indicators need to call
std::flush
to update the output stream, and doing so repeatedly with small intervals is not very efficient.I wanted to open a discussion about if and how this might be mitigated in the library. I'm trying to think of solutions that don't involve threads, since that's not necessarily portable across platforms.
My initial idea would be to add an option
UpdateType
with valuesEager
orInterval
. The former would update the stream as soon as progress is updated (the current behavior). The second would only update if a specific time interval (another option,UpdateInterval
) has elapsed since the previous update. This should improve performance for all processes which tick faster than the specified interval. However, for processes which tick slower than the update interval, you'll probably end up with some sort of update aliasing, which I think would manifest as irregular updating? In that case, though, I don't know why you wouldn't just useEager
mode. Also, this probably would not be useful for the Indeterminate Progress Bar.And in case this needs to be said, if this or some other idea seem reasonable and within scope, I'd be happy to contribute them.
The text was updated successfully, but these errors were encountered: