Re: [PATCH v2] Limit file descriptors used by packs

[Erik Faye-Lund <kusmabite@gmail.com>]

On Mon, Feb 28, 2011 at 9:52 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> 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<---