Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkgin requires way too much RAM #9

Closed
jperkin opened this issue Oct 18, 2013 · 4 comments
Closed

pkgin requires way too much RAM #9

jperkin opened this issue Oct 18, 2013 · 4 comments
Assignees

Comments

@jperkin
Copy link
Contributor

jperkin commented Oct 18, 2013

Whilst building our 2013Q3 package sets, I noticed that an initial pkgin up with an empty database failed on zones with 256M RAM:

processing remote summary (http://pkgsrc.joyent.com/packages/SmartOS/2013Q3/x86_64/All)...
updating database:  88%pkgin: can't allocate memory
: Not enough space

Increasing the zone memory to 512M and retrying, I monitored the RSS usage and it required 271M to complete the update.

Looking through the code, there is an awful lot of caching and over-allocation going on. I have an initial diff to at least stream the DB updates:

$NetBSD$

--- summary.c.orig  2013-06-16 14:00:45.000000000 +0000
+++ summary.c
@@ -87,7 +87,7 @@ int               colnames(void *, int, char **, ch

 char       *env_repos, **pkg_repos;
 char       **commit_list = NULL;
-int            commit_idx = 0;
+int            commit_idx = -1;
 int            query_size = BUFSIZ;
 /* column count for table fields, given by colnames callback */
 int            colcount = 0;
@@ -398,6 +398,24 @@ update_col(struct Summary sum, int pkgid
 /* default version for (rare and buggy) packages with a version */
 #define NOVERSION "-0.0"

+void
+do_commit_list()
+{
+   int i;
+
+   commit_idx++;
+   XREALLOC(commit_list, (commit_idx + 1) * sizeof(char *));
+   commit_list[commit_idx] = NULL;
+
+   for (i = 0; commit_list[i] != NULL; i++) {
+       pkgindb_doquery(commit_list[i], NULL, NULL);
+   }
+
+   free_list(commit_list);
+   commit_list = NULL;
+   commit_idx = -1;
+}
+
 static void
 insert_summary(struct Summary sum, char **summary, char *cur_repo)
 {
@@ -416,11 +434,9 @@ insert_summary(struct Summary sum, char
    /* record columns names to cols */
    pkgindb_doquery(query, colnames, NULL);

-   SLIST_INIT(&inserthead);
-
-   XMALLOC(commit_list, sizeof(char *));
    /* begin transaction */
-   XSTRDUP(commit_list[0], "BEGIN;");
+   snprintf(query, BUFSIZ, "BEGIN;");
+   pkgindb_doquery(query, NULL, NULL);

    printf(MSG_UPDATING_DB);
    fflush(stdout);
@@ -452,6 +468,8 @@ insert_summary(struct Summary sum, char
                pkgname = tmpname;
            }

+           SLIST_INIT(&inserthead);
+
            add_to_slist("FULLPKGNAME", pkgname);

            /* split PKGNAME and VERSION */
@@ -476,6 +494,9 @@ insert_summary(struct Summary sum, char
        /* build INSERT query */
        prepare_insert(pkgid, sum, cur_repo);

+       /* Perform the query */
+       do_commit_list();
+
        /* next PKG_ID */
        pkgid++;

@@ -487,20 +508,12 @@ insert_summary(struct Summary sum, char

    } /* while *psum != NULL */

-   commit_idx++;
-   XREALLOC(commit_list, (commit_idx + 2) * sizeof(char *));
-   XSTRDUP(commit_list[commit_idx], "COMMIT;");
-   commit_list[commit_idx + 1] = NULL;
-
-   /* do the insert */
-   for (i = 0; commit_list[i] != NULL; i++)
-       pkgindb_doquery(commit_list[i], NULL, NULL);
+   /* finish the transaction */
+   snprintf(query, BUFSIZ, "COMMIT;");
+   pkgindb_doquery(query, NULL, NULL);

    progress(alnum[strlen(alnum) - 1]); /* XXX: nasty. */

-   free_list(commit_list);
-   commit_idx = 0;
-
    /* reset pkgid */
    if (sum.type == LOCAL_SUMMARY)
        pkgid = 1;

but this could do with cleaning up further - it still requires 70MB, which seems like an awful lot when the uncompressed pkg_summary is only 14MB.

@iMilnb
Copy link
Contributor

iMilnb commented Apr 7, 2014

Hi Jonathan,

Thanks for this patch, could you make it a pull request?

@khorben
Copy link
Member

khorben commented May 27, 2015

Was this ever integrated? It doesn't apply anymore.

@jperkin
Copy link
Contributor Author

jperkin commented May 28, 2015

No, it wasn't integrated. Patch against trunk is at TritonDataCenter/pkgsrc@7fee6a0

@jperkin jperkin self-assigned this Aug 13, 2015
@jperkin
Copy link
Contributor Author

jperkin commented Aug 13, 2015

A better fix has been integrated as part of #58.

@jperkin jperkin closed this as completed Aug 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants