Hi all,
[I posted a similar proposal under the thread "How to generate GNU make
dependency file for a SPEC file automatically?" but I'm reposting here with
a better subject]
Proposal: add a new option to rpmbuild, something like
--generate-md5sum= that writes the md5sum (or whatever other hash, s
In case maximum number of open files limit is set too high, Pexec() spends way
too much time trying to set FD_CLOEXEC for all those file descriptors,
resulting in severe increase of time it takes to execute rpm or dnf. For
example, this happens while running under Docker because:
> $ docker run
glib code here:
https://gitlab.gnome.org/GNOME/glib/blob/487b1fd20c5e494366a82ddc0fa6b53b8bd779ad/glib/gspawn.c#L1207
Also worth looking at what systemd does.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.c
At a quick glance anyways, your code looks fine to me.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/444#issuecomment-389980176
Saur2000 requested changes on this pull request.
> + struct dirent *entry;
+ while ((entry = readdir(dir)) != NULL) {
+ fd = atoi(entry->d_name);
+ if (fd >= min_fd)
+ set_cloexec(fd);
+ }
+ closedir(dir);
+
+ return;
The CI failure on rawhide is unrelated, it's a dnf error complaining about repo
metadata:
> raise ValueError("The supplied metadata version isn't supported")
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://gith
kolyshkin commented on this pull request.
> + struct dirent *entry;
+ while ((entry = readdir(dir)) != NULL) {
+ fd = atoi(entry->d_name);
+ if (fd >= min_fd)
+ set_cloexec(fd);
+ }
+ closedir(dir);
+
+ return;
+
+fall
kolyshkin commented on this pull request.
> @@ -330,26 +330,64 @@ static int Pmkfifo(lua_State *L)/**
> mkfifo(path) */
}
+static void set_cloexec(int fd)
+{
+ int flag = fcntl(fd, F_GETFD);
+
+ if (flag == -1 || (flag & FD_CLOEXEC))
+ return;
+
+
kolyshkin commented on this pull request.
> + fcntl(fd, F_SETFD, FD_CLOEXEC);
+}
+
+static void set_cloexec_all(void)
+{
+ const int min_fd = 3; // don't touch stdin/out/err
+ const int default_open_max = 1024;
+ int fd, open_max;
+
+ open_max = sysconf(_SC_OPEN_MAX);
I did some more benchmarks comparing the new /proc way with the old one. It
seems that the old one is faster when `open_max < 60` which should almost never
be the case. Based on that and the suggestion from @Saur2000 I'm dropping the
conditional switch to old method completely.
--
You are rece
```
kir@kd:~/git/rpm$ time ./a 64 100 p
real0m3.413s
user0m0.416s
sys 0m2.994s
kir@kd:~/git/rpm$ time ./a 64 100 i
real0m3.661s
user0m1.421s
sys 0m2.233s
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view
For the reference, here's my quick-n-dirty benchmarking code:
```c
#include
#include
#include
#include
#include
#include
static void set_cloexec(int fd)
{
int flags = fcntl(fd, F_GETFD);
if (flags == -1 || (flags & FD_CLOEXEC))
return;
fcntl(fd, F_
OK, patch updated to take all the review comments into account (thanks
@Saur2000 for your suggestions) so it's now also fixing the `doScriptExec()`
from `lib/rpmscript.c`.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
htt
@cgwalters I took a look at glib implementation, they make use of `fdwalk()` if
available (it's a function unique to SunOS/Solaris AFAICS), and reimplement it
using /proc/self/fd if not (which is the case for Linux). This seems to be an
unnecessary complication (unless we care much about Solari
> One other thing is, they call getrlimit(RLIMIT_NOFILE, &rl) and use
> rl.rlim_max which seems to be a mistake -- rlim_cur should be used.
Filed https://bugzilla.gnome.org/show_bug.cgi?id=796227
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly o
> Filed https://bugzilla.gnome.org/show_bug.cgi?id=796227
This one is bad, as ulimit might have been changed during the runtime of a
process, and so using rlim_max is more correct and safe. For the same reason I
have added the second commit
https://github.com/rpm-software-management/rpm/pull/44
16 matches
Mail list logo