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.

Sign up for GitHub

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

Jump to bottom

Use Gmainloop as a main loop to increase performance #1943

Merged
merged 4 commits into from
Jan 16, 2024
Merged

Use Gmainloop as a main loop to increase performance #1943

merged 4 commits into from
Jan 16, 2024

Conversation

H3rnand3zzz
Copy link
Contributor

@H3rnand3zzz H3rnand3zzz commented Dec 28, 2023 •
edited
Loading

Potential improvements:

  • Further testing to make sure that no additional issues are introduced.
  • I ran valgrind when using my new feature

Current version significantly improves performance, especially noticeable on alt+scroll.

rodarima and others added 3 commits December 28, 2023 18:09
@rodarima @H3rnand3zzz
Run the main loop with GMainLoop
f995944
@rodarima @H3rnand3zzz
Implement keyboard input using GIOChannel
7eac636
@H3rnand3zzz
Fix chat state updating
f322639 
Previous commits introduced a problem that chat state
stopped working, this commit resolves it by updating it on each cycle.
@H3rnand3zzz H3rnand3zzz changed the title Gmainloop Use Gmainloop as a main loop to increase performance Dec 28, 2023
@H3rnand3zzz H3rnand3zzz mentioned this pull request Dec 28, 2023
Open
@jubalh jubalh added this to the next milestone Dec 29, 2023
@jubalh
Copy link
Member

jubalh commented Dec 29, 2023

Thanks for working on this!

Started in #1629.
@rodarima could you also review?

@rodarima
Copy link
Contributor

I gave it a quick look and looks good, but I haven't tested it. Thanks for resurrecting this :-)

@jubalh
Copy link
Member

jubalh commented Jan 8, 2024

I'm running this on my local machine since today, so far it seems to work fine.

@H3rnand3zzz did you measure any performance improvements?

I wonder how much it gives us so far, and without strophe/libstrophe#191.

We also should link to #1246

@H3rnand3zzz
Copy link
Contributor Author

I'm running this on my local machine since today, so far it seems to work fine.

@H3rnand3zzz did you measure any performance improvements?

I wonder how much it gives us so far, and without strophe/libstrophe#191.

We also should link to #1246

It's very noticeable, especially with alt+scrolling, though I did not exactly measured it with timings.

@H3rnand3zzz
Copy link
Contributor Author

@sjaeckel @jubalh any updates?

@jubalh
Copy link
Member

jubalh commented Jan 16, 2024

I ran this since a couple of days and didn't find any issues.
But I also have only a limited usecase. Let's merge it and see what others find. In the worst case we will revert back.

@jubalh jubalh merged commit 609fde0 into profanity-im:master Jan 16, 2024
6 checks passed
@H3rnand3zzz H3rnand3zzz mentioned this pull request Jan 18, 2024
Closed
H3rnand3zzz added a commit to H3rnand3zzz/profanity that referenced this pull request Jan 18, 2024
@H3rnand3zzz
Remove unnecessary input win redisplay
6b2e1b2 
The redisplay function in keyboard handling event was introduced by @rodarima
in the following commit:
7eac636

At the point first commit introduction it made sense, but later
the following commit added proper handling of input redisplay.
ccede06

This change was overlooked by me and introduced in profanity-im#1943.

Potential solution for profanity-im#1947.
@ike08 ike08 mentioned this pull request Feb 4, 2024
Closed
jubalh added a commit that referenced this pull request Feb 18, 2024
@jubalh
Fix slashguard
9b193b1 
Slashguard wasn't working since merging
#1943.

In 7eac636 the user input processing
was moved to be in gmainloop via inp_add_watch().

Fix #1955
jubalh added a commit that referenced this pull request Feb 19, 2024
@jubalh
Undo gmainloop related changes
88b26cf 
Revert "Merge pull request #1943 from H3rnand3zzz/gmainloop

This reverts commit 609fde0, reversing
changes made to 2ec9406.

Revert "Merge pull request #1948 from H3rnand3zzz/fix/rl-less-refreshes"

This reverts commit 11762fd, reversing
changes made to 609fde0.

We have got several issues, that we don't quite see how to solve, with
the merge of the gmainloop PR.

* Slashguard is broken (#1955) (though #1956 could fix that)
* One person reported problems with copy paste selection via mouse
* Some input buffer seems not to be cleared correctly
  It happened that I was debugging profanity used `/connect` and typed
  the password. I then debugged so long that a time out occurred, so
  profanity disconnected. Then it printed "unknown command: $password".

There was something else that I forgot now.

Bottomline is: so far we didn't get it right so we will undo these
changes until someone proposes a working solution.

We got a slight performance increase (apparently noticable when
alt+mouse scrolling) but got too many issues with this change.
@jubalh
Copy link
Member

jubalh commented Feb 19, 2024

Undone in 88b26cf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjaeckel sjaeckel Awaiting requested review from sjaeckel

@jubalh jubalh Awaiting requested review from jubalh

Assignees

@H3rnand3zzz H3rnand3zzz

Labels
improvement
Projects
None yet
Milestone
next
Development

Successfully merging this pull request may close these issues.
3 participants
@H3rnand3zzz @jubalh @rodarima