gnuastro-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[gnuastro-commits] master 9d4ae887 1/2: Library (checkset): also using M


From: Mohammad Akhlaghi
Subject: [gnuastro-commits] master 9d4ae887 1/2: Library (checkset): also using MemFree to find available memory
Date: Tue, 12 Apr 2022 20:23:56 -0400 (EDT)

branch: master
commit 9d4ae887637de0c26770a062ede0a3249d42560b
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Library (checkset): also using MemFree to find available memory
    
    Until now, to find the available memory, Gnuastro's memory allocation
    library would only check the 'MemAvailable' keyword in
    '/proc/meminfo'. However, on some systems (tested on Ubuntu within the
    Windows Subsystem for Linux), only has the 'MemFree' keyword is
    available. On most other native GNU/Linux systems, both these keywords are
    available.
    
    Therefore, when reading a large file on the former type of operating
    systems, Gnuastro programs will print a warning (and would not be able to
    find how much available memory there is!). When there was no problem (space
    was actually available), the program would finish, but this warning would
    be annoying for the user. In a worst case, this inability would lead to a
    crash (due to a lack of memory).
    
    With this commit, the part that reads the keyword and its value is now
    separated (modularized/abstracted) into a separate function. Thanks to the
    newly spinned-off function, Gnuastro now checks for both keywords.
    
    In fact, after reading more about the difference between these keywords (in
    [1]), I noticed that 'MemFree' is the actual physical available memory,
    while 'MemAvaialble' is the estimated (not necessarily actually free)
    space. So 'MemFree' is actually the better keyword to check first. As a
    result, we now check 'MemFree' and if it doesn't exist, we'll check
    'MemAvailable'.
    
    On a sidenote, I noticed that the spacing between lines in one part of
    'bin/mkcatalog/ui.c' was not correct, so I added a new empty line (this was
    not worth a separate commit, so I am committing it with this!).
    
    This bug was reported by Juan Miro.
    
    This fixes bug #62305.
    
    [1] 
https://lynxbee.com/understanding-proc-meminfo-analyzing-linux-memory-utilization/#.YlYJgR5ByXI
---
 NEWS               |   3 +
 bin/mkcatalog/ui.c |   1 +
 lib/checkset.c     | 183 +++++++++++++++++++++++++++++------------------------
 3 files changed, 105 insertions(+), 82 deletions(-)

diff --git a/NEWS b/NEWS
index 1e66dd33..986ef884 100644
--- a/NEWS
+++ b/NEWS
@@ -48,6 +48,9 @@ See the end of the file for license conditions.
   bug #62253: Arithmetic: segmentation fault when invalid input given for
               'ra-to-degree' or 'deg-to-degree' operators. Found by Pedram
               Ashofte Ardakani.
+  bug #62305: Not reading the 'MemFree' keyword in '/proc/meminfo' to find
+              the available RAM (and thus printing an annoying
+              warning). Reported by Juan Miro.
 
 
 
diff --git a/bin/mkcatalog/ui.c b/bin/mkcatalog/ui.c
index b2e5f3ab..50b0653a 100644
--- a/bin/mkcatalog/ui.c
+++ b/bin/mkcatalog/ui.c
@@ -836,6 +836,7 @@ ui_read_labels(struct mkcatalogparams *p)
           "currently only supports 2D or 3D datasets", p->objectsfile,
           p->cp.hdu, p->objects->ndim);
 
+
   /* Make sure the '--spectrum' option is not given on a 2D image.  */
   if(p->spectrum && p->objects->ndim!=3)
     error(EXIT_FAILURE, 0, "%s (hdu %s) has %zu dimensions, but '--spectrum' "
diff --git a/lib/checkset.c b/lib/checkset.c
index 11eba377..96068cf1 100644
--- a/lib/checkset.c
+++ b/lib/checkset.c
@@ -86,6 +86,90 @@ gal_checkset_gsl_rng(uint8_t envseed_bool, const char **name,
 
 
 
+static size_t
+checkset_meminfo_line(char *line, char *keyname, size_t keylen,
+                      char *file)
+{
+  size_t *freemem=NULL, out=GAL_BLANK_SIZE_T;
+  char *linecp, *token, *units="kB", delimiters[] = " ", *saveptr;
+
+  if( !strncmp(line, keyname, keylen) )
+    {
+      /* We need to work on a copied line to avoid messing up the
+         contents of the actual line. */
+      gal_checkset_allocate_copy(line, &linecp);
+
+      /* The first token (which we don't need). */
+      token=strtok_r(linecp, delimiters, &saveptr);
+
+      /* The second token (which is the actual number we want). */
+      token=strtok_r(NULL, delimiters, &saveptr);
+      if(token)
+        {
+          /* Read the token as a number. */
+          if( gal_type_from_string((void **)(&freemem), token,
+                                   GAL_TYPE_SIZE_T) )
+            error(EXIT_SUCCESS, 0, "WARNING: %s: value of '%s' "
+                  "keyword couldn't be read as an integer. Hence "
+                  "the amount of available RAM couldn't be "
+                  "determined. If a large volume of data is "
+                  "provided, the program may crash. Please contact "
+                  "us at '%s' to fix the problem",
+                  file, keyname, PACKAGE_BUGREPORT);
+          else
+            {
+              /* The third token should be the units ('kB'). If it
+                 isn't, there should be an error because we currently
+                 assume kilobytes. */
+              token=strtok_r(NULL, delimiters, &saveptr);
+              if(token)
+                {
+                  /* The units should be 'kB' (for kilobytes). */
+                  if( !strncmp(token, units, 2) )
+                    out=freemem[0]*1000;
+                  else
+                    error(EXIT_SUCCESS, 0, "WARNING: %s: the units of "
+                          "the value of '%s' keyword is (usually 'kB') "
+                          "isn't recognized. Hence the amount of "
+                          "available RAM couldn't be determined. If a "
+                          "large volume of data is provided, the "
+                          "program may crash. Please contact us at "
+                          "'%s' to fix the problem", file, keyname,
+                          PACKAGE_BUGREPORT);
+                }
+              else
+                error(EXIT_SUCCESS, 0, "WARNING: %s: the units of the "
+                      "value of '%s' keyword (usually 'kB') couldn't "
+                      "be read as an integer. Hence the amount of "
+                      "available RAM couldn't be determined. If a "
+                      "large volume of data is provided, the program "
+                      "may crash. Please contact us at '%s' to fix "
+                      "the problem", file, keyname, PACKAGE_BUGREPORT);
+            }
+
+          /* Clean up. */
+          if(freemem) free(freemem);
+        }
+      else
+        error(EXIT_SUCCESS, 0, "WARNING: %s: line with the '%s' "
+              "keyword didn't have a value. Hence the amount of "
+              "available RAM couldn't be determined. If a large "
+              "volume of data is provided, the program may crash. "
+              "Please contact us at '%s' to fix the problem",
+              file, keyname, PACKAGE_BUGREPORT);
+
+      /* Clean up. */
+      free(linecp);
+    }
+
+  /* Return the value. */
+  return out;
+}
+
+
+
+
+
 /* On the Linux kernel, due to "overcommitting" (which is activated by
    default), malloc will not return NULL when we allocate more memory than
    the physically available memory. It is possible to disable overcommiting
@@ -103,11 +187,10 @@ size_t
 gal_checkset_ram_available(int quietmmap)
 {
   FILE *file;
-  int keyfound=0;
-  size_t *freemem=NULL;
+  char *meminfo="/proc/meminfo", *line;
   size_t linelen=80, out=GAL_BLANK_SIZE_T;
-  char *token, *line, *linecp, *saveptr, delimiters[] = " ";
-  char *meminfo="/proc/meminfo", *keyname="MemAvailable", *units="kB";
+  char *key2="MemFree", *key1="MemAvailable";
+  size_t key1len=strlen(key1), key2len=strlen(key2);
 
   /* If /proc/meminfo exists, read it. Otherwise, don't bother doing
      anything. */
@@ -121,88 +204,24 @@ gal_checkset_ram_available(int quietmmap)
               __func__, linelen*sizeof *line);
 
       /* Read it line-by-line until you find 'MemAvailable'.  */
-      while( getline(&line, &linelen, file) != -1 )
-        if( !strncmp(line, keyname, 12) )
-          {
-            /* Necessary for final check: */
-            keyfound=1;
-
-            /* We need to work on a copied line to avoid messing up the
-               contents of the actual line. */
-            gal_checkset_allocate_copy(line, &linecp);
-
-            /* The first token (which we don't need). */
-            token=strtok_r(linecp, delimiters, &saveptr);
-
-            /* The second token (which is the actual number we want). */
-            token=strtok_r(NULL, delimiters, &saveptr);
-            if(token)
-              {
-                /* Read the token as a number. */
-                if( gal_type_from_string((void **)(&freemem), token,
-                                         GAL_TYPE_SIZE_T) )
-                  error(EXIT_SUCCESS, 0, "WARNING: %s: value of '%s' "
-                        "keyword couldn't be read as an integer. Hence "
-                        "the amount of available RAM couldn't be "
-                        "determined. If a large volume of data is "
-                        "provided, the program may crash. Please contact "
-                        "us at '%s' to fix the problem",
-                        meminfo, keyname, PACKAGE_BUGREPORT);
-                else
-                  {
-                    /* The third token should be the units ('kB'). If it
-                       isn't, there should be an error because we currently
-                       assume kilobytes. */
-                    token=strtok_r(NULL, delimiters, &saveptr);
-                    if(token)
-                      {
-                        /* The units should be 'kB' (for kilobytes). */
-                        if( !strncmp(token, units, 2) )
-                          out=freemem[0]*1000;
-                        else
-                          error(EXIT_SUCCESS, 0, "WARNING: %s: the units of "
-                                "the value of '%s' keyword is (usually 'kB') "
-                                "isn't recognized. Hence the amount of "
-                                "available RAM couldn't be determined. If a "
-                                "large volume of data is provided, the "
-                                "program may crash. Please contact us at "
-                                "'%s' to fix the problem", meminfo, keyname,
-                                PACKAGE_BUGREPORT);
-                      }
-                    else
-                      error(EXIT_SUCCESS, 0, "WARNING: %s: the units of the "
-                            "value of '%s' keyword (usually 'kB') couldn't "
-                            "be read as an integer. Hence the amount of "
-                            "available RAM couldn't be determined. If a "
-                            "large volume of data is provided, the program "
-                            "may crash. Please contact us at '%s' to fix "
-                            "the problem", meminfo, keyname, 
PACKAGE_BUGREPORT);
-                  }
-
-                /* Clean up. */
-                if(freemem) free(freemem);
-              }
-            else
-              error(EXIT_SUCCESS, 0, "WARNING: %s: line with the '%s' "
-                    "keyword didn't have a value. Hence the amount of "
-                    "available RAM couldn't be determined. If a large "
-                    "volume of data is provided, the program may crash. "
-                    "Please contact us at '%s' to fix the problem",
-                    meminfo, keyname, PACKAGE_BUGREPORT);
-
-            /* Clean up. */
-            free(linecp);
-          }
+      while( getline(&line, &linelen, file) != -1
+             && out == GAL_BLANK_SIZE_T )
+        {
+          out=checkset_meminfo_line(line, key1, key1len, meminfo);
+          if( out == GAL_BLANK_SIZE_T )
+            out=checkset_meminfo_line(line, key2, key2len, meminfo);
+        }
 
       /* The file existed but a keyname couldn't be found. In this case we
          should inform the user to be aware that we can't automatically
          determine the available memory. */
-      if(keyfound==0 && quietmmap==0)
-        error(EXIT_SUCCESS, 0, "WARNING: %s: didn't contain a '%s' keyword "
-              "hence the amount of available RAM couldn't be determined. "
-              "If a large volume of data is provided, the program may "
-              "crash. Please contact us at '%s' to fix the problem",
-              meminfo, keyname, PACKAGE_BUGREPORT);
+      if(out==GAL_BLANK_SIZE_T && quietmmap==0)
+        error(EXIT_SUCCESS, 0, "WARNING: %s: didn't contain a '%s' "
+              "or '%s' keywords hence the amount of available RAM "
+              "couldn't be determined. If a large volume of data is "
+              "provided, the program may crash. Please contact us at "
+              "'%s' to fix the problem", meminfo, key1, key2,
+              PACKAGE_BUGREPORT);
 
       /* Close the opened file and free the line. */
       free(line);



reply via email to

[Prev in Thread] Current Thread [Next in Thread]