garbage collection changes

CHANGELOG

@@ -7,6 +7,13 @@ v3.0.0 (XXXX-XX-XX)
 v2.8.0 (XXXX-XX-XX)
 -------------------
 
+* slightly adjusted V8 garbage collection strategy so that collection eventually
+  happens in all contexts that hold V8 external references to documents and
+  collections.
+
+  also adjusted default value of `--javascript.gc-frequency` from 10 seconds to
+  15 seconds, as less internal operations are carried out in JavaScript.
+
 * fixes for AQL optimizer and traversal
 
 * added `--create-collection-type` option to arangoimp
@@ -311,9 +318,9 @@ v2.8.0-alpha1 (2015-12-03)
 v2.7.4 (XXXX-XX-XX)
 -------------------
 
-* randomly shuffle available V8 contexts when picking one for an HTTP or scheduler
+* slightly adjusted V8 garbage collection strategy so that collection eventually
-  task. This distributes requests more evenly across contexts so they can eventually
+  happens in all contexts that hold V8 external references to documents and
-  be garbage-collected.
+  collections.
 
 * added the following attributes to the result of `collection.figures()` and the
   corresponding HTTP API at `PUT /_api/collection/<name>/figures`:

arangod/V8Server/ApplicationV8.cpp

@@ -297,7 +297,7 @@ ApplicationV8::ApplicationV8 (TRI_server_t* server,
     _useActions(true),
     _frontendVersionCheck(true),
     _gcInterval(1000),
-    _gcFrequency(10.0),
+    _gcFrequency(15.0),
     _v8Options(""),
     _startupLoader(),
     _vocbase(nullptr),
@@ -442,9 +442,6 @@ ApplicationV8::V8Context* ApplicationV8::enterContext (TRI_vocbase_t* vocbase,
     LOG_TRACE("found unused V8 context");
     TRI_ASSERT(! _freeContexts.empty());
    
-    // randomly shuffle free contexts so that different get contexts get used (and garbage collected) 
-    std::random_shuffle(_freeContexts.begin(), _freeContexts.end());
-
     context = _freeContexts.back();
     TRI_ASSERT(context != nullptr);
 
@@ -629,6 +626,7 @@ void ApplicationV8::collectGarbage () {
   // to false again once all contexts have been cleaned up and there is nothing
   // more to do
   volatile bool useReducedWait = false;
+  bool preferFree = false;
 
   // the time we'll wait for a signal
   uint64_t const regularWaitTime = (uint64_t) (_gcFrequency * 1000.0 * 1000.0);
@@ -638,9 +636,11 @@ void ApplicationV8::collectGarbage () {
 
   while (_stopping == 0) {
     V8Context* context = nullptr;
+    bool wasDirty = false;
 
     {
       bool gotSignal = false;
+      preferFree = ! preferFree;
       CONDITION_LOCKER(guard, _contextCondition);
 
       if (_dirtyContexts.empty()) {
@@ -653,26 +653,39 @@ void ApplicationV8::collectGarbage () {
         // the reduced wait time will allow to perfom GC for more contexts
         useReducedWait = ! gotSignal;
       }
+      
+      if (preferFree && ! _freeContexts.empty()) {
+        context = pickFreeContextForGc();
+      }
 
-      if (! _dirtyContexts.empty()) {
+      if (context == nullptr && 
+          ! _dirtyContexts.empty()) {
         context = _dirtyContexts.back();
         _dirtyContexts.pop_back();
-        useReducedWait = false;
+        if (context->_numExecutions < 10 && ! context->_hasActiveExternals) {
+          // don't collect this one
+          _freeContexts.emplace_back(context);
+          context = nullptr;
+        }
+        else {
+          wasDirty = true;
+        }
       }
-      else if (! gotSignal && ! _freeContexts.empty()) {
+
+      if (context == nullptr && 
+          ! preferFree && 
+          ! gotSignal && 
+          ! _freeContexts.empty()) {
         // we timed out waiting for a signal, so we have idle time that we can
         // spend on running the GC pro-actively
         // We'll pick one of the free contexts and clean it up
         context = pickFreeContextForGc();
+      }
 
-        // there is no context to clean up, probably they all have been cleaned up
+      // there is no context to clean up, probably they all have been cleaned up
-        // already. increase the wait time so we don't cycle too much in the GC loop
+      // already. increase the wait time so we don't cycle too much in the GC loop
-        // and waste CPU unnecessary
+      // and waste CPU unnecessary
-        useReducedWait = (context != nullptr);
+      useReducedWait = (context != nullptr);
-      }
-      else {
-        useReducedWait = false; 
-      }
     }
 
     // update last gc time
@@ -680,6 +693,7 @@ void ApplicationV8::collectGarbage () {
     gc->updateGcStamp(lastGc);
 
     if (context != nullptr) {
+      // LOG_TRACE("will collect now: %d, numExecutions: %d, hasActive: %d", (int) context->_id, (int) context->_numExecutions, (int) context->_hasActiveExternals);
       LOG_TRACE("collecting V8 garbage");
       bool hasActiveExternals = false;
       auto isolate = context->isolate;
@@ -716,10 +730,18 @@ void ApplicationV8::collectGarbage () {
       {
         CONDITION_LOCKER(guard, _contextCondition);
 
-        _freeContexts.emplace_back(context);
+        if (wasDirty) {
+          _freeContexts.emplace_back(context);
+        }
+        else {
+          _freeContexts.insert(_freeContexts.begin(), context);
+        }
         guard.broadcast();
       }
     }
+    else {
+      useReducedWait = false; // sanity
+    }
   }
 
   _gcFinished = true;
@@ -1188,9 +1210,9 @@ ApplicationV8::V8Context* ApplicationV8::pickFreeContextForGc () {
   int pickedContextNr = -1; // index of context with lowest GC stamp, -1 means "none"
 
 
-  for (int i = 0; i < n; ++i) {
+  for (int i = n - 1; i > 0; --i) {
     // check if there's actually anything to clean up in the context
-    if (_freeContexts[i]->_numExecutions == 0 && ! _freeContexts[i]->_hasActiveExternals) {
+    if (_freeContexts[i]->_numExecutions < 10 && ! _freeContexts[i]->_hasActiveExternals) {
       continue;
     }
 
@@ -1253,6 +1275,7 @@ bool ApplicationV8::prepareV8Instance (size_t i, bool useActions) {
   TRI_ASSERT(context->_locker == nullptr);
 
   // enter a new isolate
+  bool hasActiveExternals = false;
   context->_id = i;
   context->isolate = isolate;
   TRI_ASSERT(context->_locker == nullptr);
@@ -1343,6 +1366,8 @@ bool ApplicationV8::prepareV8Instance (size_t i, bool useActions) {
           break;
       }
     }
+    TRI_GET_GLOBALS();
+    hasActiveExternals = v8g->hasActiveExternals();
 
     // and return from the context
     localContext->Exit();
@@ -1357,7 +1382,7 @@ bool ApplicationV8::prepareV8Instance (size_t i, bool useActions) {
 
   // initialize garbage collection for context
   context->_numExecutions      = 0;
-  context->_hasActiveExternals = true;
+  context->_hasActiveExternals = hasActiveExternals;
   context->_lastGcStamp        = TRI_microtime() + randomWait;
 
   LOG_TRACE("initialized V8 context #%d", (int) i);