Merge lp://staging/~radu.c/glipper/clipboard-watch into lp://staging/glipper

Proposed by Radu Cristescu
Status: Needs review
Proposed branch: lp://staging/~radu.c/glipper/clipboard-watch
Merge into: lp://staging/glipper
Diff against target: 93 lines (+45/-4)
1 file modified
glipper/Clipboards.py (+45/-4)
To merge this branch: bzr merge lp://staging/~radu.c/glipper/clipboard-watch
Reviewer Review Type Date Requested Status
Laszlo Pandy Needs Fixing
Review via email: mp+59041@code.staging.launchpad.net

Commit message

Poll the clipboard regularly instead of relying on the 'owner-change' signal. Clipboard handling still works OK, but glipper will no longer use 100% CPU if an application (like pidgin) misbehaves and bombards the clipboard with changes.

Description of the change

I made a change to how clipboard changes are handled because of how wait_for_text() works. Thing is, if the clipboard is bombarded with changes, say hundreds of changes per second for a few seconds, then GTK spawns a lot of on_clipboard_owner_change instances, which in turn call wait_for_text(), because wait_for_text() actually suspends the function and returns control to the main loop, which can send more "owner change" events out. This also hogs the CPU, which is very annoying.

In this branch I poll the clipboard every half second instead. It looks like the function added with timeout_add won't be executed multiple times, but instead GTK waits to see what the function has to say: call this function again, or don't call this function again (and delete the timer).

If you need additional information, please let me know.

To post a comment you must log in.
Revision history for this message
Laszlo Pandy (laszlok) wrote :

I'm not sure I like the approach of polling.

The purpose is to prevent overuse of CPU when the clipboard is being changed rapidly. However I don't think this is a very common case.

If there was no disadvantage to this approach, it would still be good to protect against this rare case. However using polling increases the CPU in the most common case -- when there are no clipboard changes. Also, using timeout callbacks adds registers a counter with the kernel, and required the CPU to wake up more frequently, preventing power saving.

I am not sure there is a great impact on the power saving, but I would still like to avoid polling when nothing is happening.

Since wait_for_text() re-enters the mainloop, would it be possible to detect if we're re-entering on_clipboard_owner_change() (maybe using a boolean) and return immediately? Would that solve the problem, or would the 'owner-change' signal firing simply cause 100% CPU anyway?

Another approach might be to have the signal handler of 'owner-change' use gobject.idle_add() with a function which actually gets the text and does the work. Then that function wouldn't be called until after all the events have fired, and you could make sure you only do the work once.

review: Needs Fixing
96. By Radu Cristescu

merge upstream

97. By Radu Cristescu

merged upstream

98. By Radu Cristescu

read clipboard contents as soon as possible, without polling, but also having a maximum of one request for clipboard text at any one time

99. By Radu Cristescu

request clipboard contents on the main loop to avoid threaded X weirdness

Unmerged revisions

99. By Radu Cristescu

request clipboard contents on the main loop to avoid threaded X weirdness

98. By Radu Cristescu

read clipboard contents as soon as possible, without polling, but also having a maximum of one request for clipboard text at any one time

97. By Radu Cristescu

merged upstream

96. By Radu Cristescu

merge upstream

95. By Radu Cristescu

use a variable to store the polling interval

94. By Radu Cristescu

reduce polling interval

93. By Radu Cristescu

fix a function call

92. By Radu Cristescu

use polling instead of notifications

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: