- Replace tabs by spaces (the other codebases I work on all use tabs :)
authorMatthew Mondor <mmondor@pulsar-zone.net>
Fri, 28 Aug 2015 22:44:05 +0000 (18:44 -0400)
committerMatthew Mondor <mmondor@pulsar-zone.net>
Fri, 28 Aug 2015 22:44:05 +0000 (18:44 -0400)
- After looking at grusage() it seems that we cannot use this to adapt heap_gap
- heap_gap demonstrated to be necessary, at least for now, now set at 50MB
- Minor cleanups

To test:

- Reduce the heap size limit by reducing the soft RLIMIT_DATA of
  your current shell.  On sh, this can be done using ulimit -d
  <kilobytes>.  Either reduce it smaller than the ECL default heap
  (1GB on 32-bit, 4GB on 64-bit), or specify a larger heap limit to
  ECL starting it with the --heap-size <bytes> option (but note that
  because of another bug it may be difficult to increase it very
  high).
- Start ECL, notice the HEAP-WARNING: messages which should show
  that the soft limit was increased to satisfy the heap.
- Run allocations until either ECL freezes or reports a heap error.
   the test is successful, it should report, not freeze, obviously :)
  Example test to run:

(defparameter *allocations* 16384)
(defparameter *alloc-size* 4096)
(defvar *queue* (make-array 1024
                            :adjustable t
                            :fill-pointer 0))

(defun test1 ()
  (loop
     repeat *allocations*
     do
       (vector-push-extend (make-array *alloc-size*
                                       :element-type '(unsigned-byte 8)
                                       :adjustable nil
                                       :initial-element #x00
                                       :fill-pointer *alloc-size*)
                           *queue*
                           0))) ; Grows automatically +1/2 size on ECL

Run (test1) as often as necessary (and observe heap grow in top(1) etc).

What remains to do before this branch can be merged:
- More testing, and confirmation that 50MB heap_gap is enough for others
- Removal of debug messages
- Perhaps issuing a single message about the size the heap was changed to
  and why, but only when in ECL-REPL interactive mode startup
- If possible, determining the value for heap_gap at runtime,
  instead of guessing it, but it seems tricky to do portably.

src/c/main.d

index a835040..867aaf9 100755 (executable)
@@ -66,79 +66,82 @@ const char *ecl_self;
 
 /* HEAP */
 
-/* 10MB extra for ECL itself */
-#define HEAP_GAP (10 * 1024 * 1024)
-
 #if ECL_FIXNUM_BITS <= 32
 #define HEAP_SIZE_DEFAULT (1024 * 1024 * 1024)
 #else
 #define HEAP_SIZE_DEFAULT (4096 * 1024 * 1024)
 #endif
 
-/* Could eventually be conditional on a DEBUG definition if kept */
+/* XXX Could eventually be conditional on a DEBUG definition if kept */
 static void
 w(const char *fmt, ...)
 {
-       va_list ap;
+        va_list ap;
 
-       dprintf(2, "HEAP-SIZE-WARNING: ");
-       va_start(ap, fmt);
-       (void) vdprintf(2, fmt, ap);
-       va_end(ap);
-       dprintf(2, "\n");
+        dprintf(2, "HEAP-SIZE-WARNING: ");
+        va_start(ap, fmt);
+        (void) vdprintf(2, fmt, ap);
+        va_end(ap);
+        dprintf(2, "\n");
 }
 
 /*
- * If the target heap size (1GB on 32-bit, 4GB otherwise) exceeds
- * the soft process data size rlimit, attempt to grow the soft limit.
- * If the hard limit doesn't allow it, reduce the target heap size
- * to the soft limit.  The heap will be smaller than expected, but ECL will
- * be able to report allocation errors gracefully rather than busy-looping
- * attempting to allocate even more room to report the error and encountering
- * even more allocation errors.
+ * If the target heap size exceeds the process's hard RLIMIT_DATA limit,
+ * reduce the target.  If the target exceeds the soft RLIMIT_DATA, attempt to
+ * grow the soft limit.
+ * The resulting heap might be smaller than requested, but ECL will be able to
+ * report allocation errors gracefully when it's reached, rather than
+ * busy-looping attempting to allocate even more resources to report the
+ * error.
+ * XXX It'd be nice to query the actual ECL heap requirements to set heap_gap,
+ * but this would need to be done portably.  On 32-bit i686 ECL, it seemed
+ * safe to have this value at 10MB, but on 64-bit amd64 ECL under 50MB the
+ * issue may still occur.
  */
 size_t
 fix_heap_size(size_t target)
 {
+        w("Entering fix_heap_size(%zd)", target);
+
 #if defined(HAVE_SYS_RESOURCE_H) && defined(RLIMIT_DATA)
         struct rlimit rlp;
+        size_t heap_gap = (50 * 1024 * 1024);
 
-       w("Entering fix_heap_size(%zd)", target);
         if (getrlimit(RLIMIT_DATA, &rlp) != 0) {
-               w("Cannot obtain RLIMIT_DATA, returning %zd", target);
+                w("Cannot obtain RLIMIT_DATA, returning %zd", target);
                 /* Cannot evaluate, keep target */
                 return target;
         }
 
-       /* Hard limit too low?  Reduce target if so. */
-       if (target + HEAP_GAP > rlp.rlim_max) {
-               w("Hard RLIMIT_DATA too low (%zd), reducing target to %zd",
-                 rlp.rlim_max, rlp.rlim_max - HEAP_GAP);
-               target = rlp.rlim_max - HEAP_GAP;
-       }
+        /* Hard limit too low?  Reduce target if so. */
+        if (target + heap_gap > rlp.rlim_max) {
+                w("Hard RLIMIT_DATA too low (%zd), reducing target to %zd",
+                  rlp.rlim_max, rlp.rlim_max - heap_gap);
+                target = rlp.rlim_max - heap_gap;
+        }
 
         /* Soft limit too low? */
-        if (target + HEAP_GAP > rlp.rlim_cur) {
-                size_t missing = target + HEAP_GAP - rlp.rlim_cur;
-
-               w("Soft RLIMIT_DATA too low (%zd)", rlp.rlim_cur);
-               /* Attempt to grow soft limit */
-               rlp.rlim_cur += missing;
-               w("Trying to increase soft limit to %zd",
-                 rlp.rlim_cur);
-               if (setrlimit(RLIMIT_DATA, &rlp) == 0) {
-                       w("We could increase soft limit to %zd, returning %zd",
-                         rlp.rlim_cur, target);
-                       return target;
-               } else {
-                       w("We could not grow soft limit to %zd, returning %zd",
-                         rlp.rlim_cur, rlp.rlim_cur - HEAP_GAP - missing);
-                       return (rlp.rlim_cur - HEAP_GAP - missing);
-               }
+        if (target + heap_gap > rlp.rlim_cur) {
+                size_t missing = target + heap_gap - rlp.rlim_cur;
+
+                w("Soft RLIMIT_DATA too low (%zd)", rlp.rlim_cur);
+                /* Attempt to grow soft limit */
+                rlp.rlim_cur += missing;
+                w("Trying to increase soft limit to %zd",
+                  rlp.rlim_cur);
+                if (setrlimit(RLIMIT_DATA, &rlp) == 0) {
+                        w("We could increase soft limit to %zd, returning %zd",
+                          rlp.rlim_cur, target);
+                        return target;
+                } else {
+                        w("We could not grow soft limit to %zd, returning %zd",
+                          rlp.rlim_cur, rlp.rlim_cur - heap_gap - missing);
+                        return (rlp.rlim_cur - heap_gap - missing);
+                }
         }
 
 #endif
-       w("Returning %zd", target);
+        w("Returning %zd", target);
         return target;
 }