Re: [PATCH v2] Limit file descriptors used by packs
On Mon, Feb 28, 2011 at 9:52 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
Show 72 quoted lines
> Rather than using 'errno == EMFILE' after a failed open() call
> to indicate the process is out of file descriptors and an LRU
> pack window should be closed, place a hard upper limit on the
> number of open packs based on the actual rlimit of the process.
>
> By using a hard upper limit that is below the rlimit of the current
> process it is not necessary to check for EMFILE on every single
> fd-allocating system call. Instead reserving 25 file descriptors
> makes it safe to assume the system call won't fail due to being over
> the filedescriptor limit. Here 25 is chosen as a WAG, but considers
> 3 for stdin/stdout/stderr, and at least a few for other Git code
> to operate on temporary files. An additional 20 is reserved as it
> is not known what the C library needs to perform other services on
> Git's behalf, such as nsswitch or name resolution.
>
> This fixes a case where running `git gc --auto` in a repository
> with more than 1024 packs (but an rlimit of 1024 open fds) fails
> due to the temporary output file not being able to allocate a
> file descriptor. The output file is opened by pack-objects after
> object enumeration and delta compression are done, both of which
> have already opened all of the packs and fully populated the file
> descriptor table.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> sha1_file.c | 43 ++++++++++++++++++++++++++++++-------------
> 1 files changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index d949b35..7850c18 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -418,6 +418,8 @@ static unsigned int pack_used_ctr;
> static unsigned int pack_mmap_calls;
> static unsigned int peak_pack_open_windows;
> static unsigned int pack_open_windows;
> +static unsigned int pack_open_fds;
> +static unsigned int pack_max_fds;
> static size_t peak_pack_mapped;
> static size_t pack_mapped;
> struct packed_git *packed_git;
> @@ -597,6 +599,7 @@ static int unuse_one_window(struct packed_git *current, int keep_fd)
> lru_p->windows = lru_w->next;
> if (!lru_p->windows && lru_p->pack_fd != keep_fd) {
> close(lru_p->pack_fd);
> + pack_open_fds--;
> lru_p->pack_fd = -1;
> }
> }
> @@ -681,8 +684,10 @@ void free_pack_by_name(const char *pack_name)
> if (strcmp(pack_name, p->pack_name) == 0) {
> clear_delta_base_cache();
> close_pack_windows(p);
> - if (p->pack_fd != -1)
> + if (p->pack_fd != -1) {
> close(p->pack_fd);
> + pack_open_fds--;
> + }
> close_pack_index(p);
> free(p->bad_object_sha1);
> *pp = p->next;
> @@ -708,9 +713,29 @@ static int open_packed_git_1(struct packed_git *p)
> if (!p->index_data && open_pack_index(p))
> return error("packfile %s index unavailable", p->pack_name);
>
> + if (!pack_max_fds) {
> + struct rlimit lim;
> + unsigned int max_fds;
> +
> + if (getrlimit(RLIMIT_NOFILE, &lim))
> + die_errno("cannot get RLIMIT_NOFILE");
> +We don't have getrlimit on Windows :(
I guess something like should work, but untested. Limit of 2048 taken from MSDN:
http://msdn.microsoft.com/en-us/library/6e3b887c(v=vs.71).aspx
---8<---
diff --git a/compat/mingw.h b/compat/mingw.h
index 9c00e75..9155ce3 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -234,6 +234,22 @@ int mingw_getpagesize(void);
#define getpagesize mingw_getpagesize
#endif
+struct rlimit {
+ unsigned int rlim_cur;
+};
+#define RLIMIT_NOFILE 0
+
+static inline int getrlimit(int resource, struct rlimit *rlp)
+{
+ if (resource != RLIMIT_NOFILE) {
+ errno = EINVAL;
+ return -1;
+ }
+
+ rlp->rlim_cur = 2048;
+ return 0;
+}
+
/* Use mingw_lstat() instead of lstat()/stat() and
* mingw_fstat() instead of fstat() on Windows.
*/
---8<---
Erik Faye-Lund <kusmabite@gmail.com>