Summary: | devel/leatherman: fix lengthy closure of open file descriptors | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | Ports & Packages | Reporter: | Rick <vrwmiller> | ||||||
Component: | Individual Port(s) | Assignee: | FreeBSD Puppet Team <puppet> | ||||||
Status: | Closed FIXED | ||||||||
Severity: | Affects Only Me | CC: | jch, romain | ||||||
Priority: | --- | Flags: | bugzilla:
maintainer-feedback?
(puppet) |
||||||
Version: | Latest | ||||||||
Hardware: | Any | ||||||||
OS: | Any | ||||||||
Attachments: |
|
Description
Rick
2018-10-22 17:24:21 UTC
Created attachment 198473 [details]
patch updates closure of open FDs
Created attachment 198474 [details]
Poudriere build log of devel/leatherman after patch
This log file illustrates the patched port building successfully.
Hi! Thanks for the problem report. Can you please detail what problems this patch is supposed to fix? Also, can you please fill-in a pull request on GitHub so that the change is accepted upstream, making maintaining the FreeBSD port easier for us? Thanks! The copy below summarizes the problem addressed by the patch. Will get a pull request open upstream. "On some FreeBSD systems sysconf(_SC_OPEN_MAX) returns a very large number, e.g., on one system, it consistently returns 3770208. The patched code was attempting to close all file descriptors from stderr to sysconf(_SC_OPEN_MAX), and thus ended up calling close() 3770205 times. This turns out to be quite slow for what it is trying to accomplish. Replacing this loop with the FreeBSD system call closefrom() accomplishes the same goal, essentially replacing 3.7 million close() system calls with one. We ran into this issue while running the facter 3.11 binary on a FreeBSD 11.2 system. All facter invocations ended up taking at least 3 seconds to complete, unless we disabled ruby integration. We discovered that it was spending most of that 3 seconds calling close() repeatedly." Neat! Before: romain@zappy /tmp % time facter > /dev/null facter > /dev/null 0,47s user 0,61s system 99% cpu 1,085 total romain@zappy /tmp % time facter > /dev/null facter > /dev/null 0,49s user 0,60s system 99% cpu 1,086 total romain@zappy /tmp % time facter > /dev/null facter > /dev/null 0,49s user 0,59s system 99% cpu 1,087 total After: romain@zappy /tmp % time facter > /dev/null facter > /dev/null 0,10s user 0,04s system 98% cpu 0,142 total romain@zappy /tmp % time facter > /dev/null facter > /dev/null 0,12s user 0,02s system 98% cpu 0,141 total romain@zappy /tmp % time facter > /dev/null facter > /dev/null 0,11s user 0,03s system 98% cpu 0,141 total Thanks for the details, please mention me (@smortex) and link to this issue when you open the PR on GitHub so that I can track this. closefrom(2) seems to be a BSD/Solaris extension so maybe the final patch will not be as simple as the one you provided, but I guess this patch is a must-have (and a very good starting point). I will commit it to the ports tree so that we can benefit from this fix while it is being merged upstream. A commit references this bug: Author: romain Date: Wed Oct 24 11:31:42 UTC 2018 New revision: 482901 URL: https://svnweb.freebsd.org/changeset/ports/482901 Log: Fix closure of open file descriptors Rely on closefrom(2) instead of a loop on close(2) to close all opened file descriptors. This greatly improve performances. While here, commit changes to patches generated by make makepatch. With hat: puppet PR: 232538 Reported by: vmiller@verisign.com Changes: head/devel/leatherman/Makefile head/devel/leatherman/files/patch-execution_src_posix_execution.cc head/devel/leatherman/files/patch-json__container_tests_json__container__test.cc head/devel/leatherman/files/patch-locale_src_locale.cc The fix was committed a few month ago, but I forgot to close the issue at that time. Closing now, thank you for reporting the problem and submitting this fix! |