Bug 240548

Summary: x11-toolkits/vte3: Loops over all possible file descriptors
Product: Ports & Packages Reporter: Ivan Rozhuk <rozhuk.im>
Component: Individual Port(s)Assignee: freebsd-desktop (Team) <desktop>
Status: Closed Overcome By Events    
Severity: Affects Many People CC: adridg, ajacoutot, lbartoletti, rozhuk.im
Priority: --- Keywords: needs-qa, performance
Version: LatestFlags: adridg: maintainer-feedback+
rozhuk.im: merge-quarterly?
Hardware: Any   
OS: Any   
URL: https://gitlab.gnome.org/GNOME/vte/issues/171
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236815
Attachments:
Description Flags
patch
none
reworked
none
patch rozhuk.im: maintainer-approval?

Description Ivan Rozhuk 2019-09-12 23:08:28 UTC
Created attachment 207443 [details]
patch

This is ported patch from devel/glib20: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236815
because vte3 uses copy-pasted code from glib.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-13 04:17:00 UTC
This change is best suited to submitting upstream first. Please create an issue or PR upstream, and add the issue/PR URL reference to the patch so we can track the patch locally until a future release comes out with the change

Does this *actually* depend on the change in bug 236815 being committed *before* this patch is committed? If not, please remove it from Depends On
Comment 2 Ivan Rozhuk 2019-09-14 02:02:02 UTC
Created attachment 207480 [details]
reworked
Comment 3 Ivan Rozhuk 2019-09-14 22:30:47 UTC
(In reply to Kubilay Kocak from comment #1)

https://reviews.freebsd.org/D21206 - after that no patch needed.
Comment 4 Ivan Rozhuk 2020-02-19 16:29:35 UTC
ping
Comment 5 Ivan Rozhuk 2020-04-16 07:50:28 UTC
maintainer timeout
Comment 6 Ivan Rozhuk 2020-09-25 18:11:50 UTC
Created attachment 218293 [details]
patch
Comment 7 Ivan Rozhuk 2020-12-21 03:32:42 UTC
maintainer timeout
Comment 8 Adriaan de Groot freebsd_committer freebsd_triage 2020-12-21 10:18:56 UTC
So the Phab review is accepted but not landed; in the meantime vte3 has a similar performance problem (it loops over all **possible** file descriptors and runs a callback, rather than iterating over **open** file descriptors) as xfce / mc had. That performance problem is described in the linked PR 236815 (open).

This change is accepted upstream (see linked URL)but might not be in any release yet.

I'll +1 this change and the other one, based **also** on vague memories of doing exactly this in some KDE code as well.
Comment 9 commit-hook freebsd_committer freebsd_triage 2021-05-18 11:27:10 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=fe337af49c25a5260b95f9a9ca8e6960176ed562

commit fe337af49c25a5260b95f9a9ca8e6960176ed562
Author:     Adriaan de Groot <adridg@FreeBSD.org>
AuthorDate: 2021-05-18 09:20:17 +0000
Commit:     Adriaan de Groot <adridg@FreeBSD.org>
CommitDate: 2021-05-18 11:25:02 +0000

    x11-toolkits/vte3: update to 0.64.1, latest upstream

    This is a minor update. While doing the update, I tried to pull
    in fixes from PR 240548 -- those were submitted upstream years ago,
    landed upstream, then removed again upstream in favor of fdwalk(),
    which might end up in in FreeBSD eventually. The fdwalk() call
    applies FD_CLOEXEC to all the file descriptors -- the original
    patch from the PR called closefrom(), but doing **that** makes
    consumers of vte3 (e.g. roxterm) hang / close. So just updating,
    not dealing with the issue of the fallback fdwalk() implementation
    being clunky.

    PR:             240548

 x11-toolkits/vte3/Makefile  | 2 +-
 x11-toolkits/vte3/distinfo  | 6 +++---
 x11-toolkits/vte3/pkg-plist | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
Comment 10 Adriaan de Groot freebsd_committer freebsd_triage 2021-05-18 11:31:49 UTC
This patch no longer applies (we're a half-dozen versions later I think), and upstream has swerved all over the place, and using `closefrom(fd)` in the obvious place gives me a broken roxterm, so I'm going to close this as overcome-by.
Comment 11 Ivan Rozhuk 2023-12-18 18:35:01 UTC
Current implementation of vte3 uses close_range(CLOSE_RANGE_CLOEXEC) if CLOSE_RANGE_CLOEXEC supported by OS (since 7 mar 2022 for 13/stable) and fall back to fdwalk() if not.
For FreeBSD fdwalk() is implemented as loop over all possible descriptors.

If some one can not update OS and want this fix - feel free to ask.