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

fix: calculate the duration for single-shot events #53

Closed

Conversation

mkvoya
Copy link

@mkvoya mkvoya commented Jan 25, 2021

I found several 0s-duration events when I use aw-watcher-window. (I have configured the poll_time of aw-watcher-window to 30 seconds, which may help present the issue.)

After some investigation, the reason seems to be the calculation of event duration. The calculation of the event duration is in the heartbeat_merge function. However, if an event occurs once and cannot be merged to the next event, the event will have a default duration of 0s, causing the above problem.

This MR detects such a situation and calculate the duration before sending such an event.

@ErikBjare
Copy link
Member

I appreciate you taking the time to investigate and making this PR, but I don't think this is the right way to go about fixing this issue.

The reasoning here is that if you only poll every 5s (or more) you can't really say with any reasonable degree of certainty that the new event has actually been active for the last 5 seconds. If an event is the exact same as in the last poll, however, we think it's a reasonable assumption (which is how heartbeat_merge works). We try to avoid making as many assumptions as possible at the tracking stage (we won't make the former assumption, but we do make the latter) and postpone more speculative approximations to the analysis stage.

When poll_time is set as its default, zero duration events will still be present, but they will be handled appropriately at the analysis stage where they get 'flooded' (i.e. they expand to fill small gaps around them). In the default query that's used in the web UI, the 'flooding duration' is set to 5 seconds, which matches up with the default poll_time to remove small gaps. You can read more about flooding in ActivityWatch/activitywatch#124, the documentation, and the source.

Reconfiguring poll_time is therefore not recommended (although we haven't made this clear, we'd appreciate a PR to the docs mentioning this). If you want the behavior this PR introduces in a watcher, you should modify the watcher itself and not embed the approximation assumption into aw-client directly.

@mkvoya
Copy link
Author

mkvoya commented Jan 26, 2021

I can understand your consideration of minimizing the assumptions.

My issue here is that the default poll_time in aw-watcher-window is 1 second (https://github.com/ActivityWatch/aw-watcher-window/blob/ff08c9ca7d102d76d3814b6302171ea54ca4cee1/aw_watcher_window/config.py#L10) which is too frequent for me and consumes a lot of CPU (and battery) on my Mac.

As the 0s-duration events are intended, I am okay if aw-client remains unchanged. (BTW, sorry for not noticing the discussion about flooding before, my bad.)

Now coming back to my issue, I personally can tolerate some seconds of inaccuracy caused by expanding the 0s-duration events, but I don't want the 0s-duration events to be totally ignored in the timeline and in the calculation of my total time. Considering a scenario where I am writing a document according to some web pages, the focus switches frequently between my editor and my browser, and ignoring 0s-duration event can half my recorded work time.

There seems to be several options to deal with a long poll_time in aw-watcher-window, e.g.,

  1. Adding a round-up option (say R seconds) to aw-watcher-window so that it initializes a new event with a minimal duration of R seconds.
  2. Making the flooding duration to be a configurable option on the WebUI side to allows a "smooth" display and "correct" calculation of event durations. But the problem here is that a suitable flood duration depends on the poll_time of a watcher, which may be different among 0s-duration events generated by different watchers.

So, what do you think?

@ErikBjare
Copy link
Member

ErikBjare commented Jan 26, 2021

My issue here is that the default poll_time in aw-watcher-window is 1 second which is too frequent for me and consumes a lot of CPU (and battery) on my Mac.

Honestly, I think the solution here would be to improve how aw-watcher-window works on macOS. It doesn't fundamentally need that much CPU, it's just that we've implemented it in a very inefficient way.

@xylix did some great work to improve the situation, but the PR has gotten stale: ActivityWatch/aw-watcher-window#40

I'm not opposed to the other things you suggested (especially point 2), but I'd rather just fix the problem (high CPU) at its core.

@mkvoya
Copy link
Author

mkvoya commented Jan 26, 2021

@xylix did some great work to improve the situation, but the PR has gotten stale: ActivityWatch/aw-watcher-window#40

AFAIK, xylix's PR acquires the window title using kCGWindowName, which does not work on my MacOS 11.1 (20C69).

According to the apple document, "few applications set the Quartz window name." So I doubt whether xylix's method still works.

A quick way to verify is to use the script in kenorb's stackoverflow answer and run something like

watch python3 the_script.py

in the shell. Then switch the focus to different apps and inspect the output of the script.

On my Mac, the script gets the active application name correctly but the window title is always b'Unknown'.
Tested applications include Safari, Safari Tech Preview, Emacs, Finder, and Microsoft Word.
For comparison, current ActivityWatch (which uses the AppleScript) acquires both the application name and the window title correctly.

@ErikBjare
Copy link
Member

@mkvoya The window title is 'Unknown' likely due to a permissions issue, not because the applications do not expose the information (it should be the same information as that which is fetched by the AppleScript after all).

See the updated PR and a similar issue in another application:

@mkvoya
Copy link
Author

mkvoya commented Mar 22, 2021

I'm closing this since the discussion is moved to the aw-watcher-window repo. Thanks. :)

@mkvoya mkvoya closed this Mar 22, 2021
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.

2 participants