Hi Brendan,

I managed to design a work around for this issue. I moved the class that I 
was trying to expose from C++ world to JavaScript - 
https://github.com/couchbase/eventing/blob/master/v8_consumer/src/builtin.js 
By doing this, whenever a call to "new N1qlQuery() 
<https://github.com/couchbase/eventing/blob/master/v8_consumer/src/builtin.js#L12>"
 
is made, we force the instantiation to be done in the JavaScript world, as 
opposed to calling NewInstance() which would create it in the C++ world. 
Thus, we get rid of the call to NewInstance() which causes the memory leak 
problem.

The methods for the class above - iter() 
<https://github.com/couchbase/eventing/blob/master/v8_consumer/src/n1ql.cc#L154>,
 
execQuery() 
<https://github.com/couchbase/eventing/blob/master/v8_consumer/src/n1ql.cc#L174>
 
and stopIter() 
<https://github.com/couchbase/eventing/blob/master/v8_consumer/src/n1ql.cc#L169>
 
are exposed from C++ to complete the class definition.

To summarize, just get rid of the call to NewInstance(). As a substitute, 
write the class to be exposed in JavaScript and expose the corresponding 
member functions from C++.

Thanks,
--Gautham

On Monday, 1 May 2017 19:55:27 UTC+5:30, Brendan Bates wrote:

> Hey Gautham B A,
>
> I am currently experiencing a similar issue.  I have found that using 
> NewInstance() causes a slow memory growth/leak in our application.  We 
> aggressively call LowMemoryNotification() 
> on our scripts, as they are meant to run for a very long time and accept 
> the performance tradeoff.  This should theoretically clean up stray 
> objects, but it seems that objects created via
> NewInstance() are never properly marked for GC (maybe, I don't really know 
> what's being held).  What's odd is that I can create a weak persistent 
> beneath one of these objects, and they
> get cleaned up regularly in the memory notification calls.
>
> Are you still having issues?  I might make a test program and throw it on 
> a gist and report back here.  I'd be curious as this doesn't seem like 
> acceptable behavior for the engine.
>
> I don't see an actual memory leak in the code you posted but creating 
>> a new ObjectTemplate for every query isn't very efficient. 
>>
>
> Just wanted to reply to this to say, that is an irrelevant point to this 
> discussion.  Even if you are using it sparingly, if you have a long-running 
> script that calls NewInstance(), then
> this would be a problem.
>
> On Tuesday, April 18, 2017 at 5:27:59 AM UTC-4, Gautham B A wrote:
>>
>> Yes, the memory leak in this code isn't visible as it appears. 
>> I observed the memory leak when I ran the "top" command. But when I 
>> commented out the call to "NewInstance()", there wasn't any growth in 
>> memory (which is obvious because there's nothing being created and passed 
>> onto the JS world).
>>
>> On Tuesday, 18 April 2017 14:49:11 UTC+5:30, Ben Noordhuis wrote:
>>>
>>> On Tue, Apr 18, 2017 at 9:31 AM, Gautham B A 
>>> <[email protected]> wrote: 
>>> > Thank you Ben. 
>>> > Yes, there's a try_catch.HasCaught() before running the script in 
>>> order to 
>>> > prevent the process from exiting. 
>>> > Here's the code that will cause the script to run - 
>>> > int SendUpdate(std::string value, std::string meta, 
>>> >                          std::string doc_type) { 
>>> >   v8::Locker locker(GetIsolate()); 
>>> >   v8::Isolate::Scope isolate_scope(GetIsolate()); 
>>> >   v8::HandleScope handle_scope(GetIsolate()); 
>>> > 
>>> >   v8::Local<v8::Context> context = 
>>> >       v8::Local<v8::Context>::New(GetIsolate(), context_); 
>>> >   v8::Context::Scope context_scope(context); 
>>> > 
>>> >   v8::TryCatch try_catch(GetIsolate()); 
>>> > 
>>> >   v8::Handle<v8::Value> args[2]; 
>>> >   if (doc_type.compare("json") == 0) { 
>>> >     args[0] = 
>>> >         v8::JSON::Parse(v8::String::NewFromUtf8(GetIsolate(), 
>>> > value.c_str())); 
>>> >   } else { 
>>> >     args[0] = v8::String::NewFromUtf8(GetIsolate(), value.c_str()); 
>>> >   } 
>>> > 
>>> >   args[1] = 
>>> >       v8::JSON::Parse(v8::String::NewFromUtf8(GetIsolate(), 
>>> meta.c_str())); 
>>> > 
>>> >   if (try_catch.HasCaught()) { 
>>> >     last_exception = ExceptionString(GetIsolate(), &try_catch); 
>>> >     LOG(logError) << "Last exception: " << last_exception << '\n'; 
>>> >   } 
>>> > 
>>> >   v8::Local<v8::Function> on_doc_update = 
>>> >       v8::Local<v8::Function>::New(GetIsolate(), on_update_); 
>>> >   on_doc_update->Call(context->Global(), 2, args); 
>>> > 
>>> >   if (try_catch.HasCaught()) { 
>>> >     LOG(logDebug) << "Exception message: " 
>>> >                   << ExceptionString(GetIsolate(), &try_catch) << 
>>> '\n'; 
>>> > 
>>> >     return ON_UPDATE_CALL_FAIL; 
>>> >   } 
>>> > 
>>> >   return SUCCESS; 
>>> > } 
>>> > 
>>> > Is it possible to reclaim the memory without shutting the VM down? 
>>>
>>> Depends on what you mean by 'reclaim' and 'memory leak' - memory isn't 
>>> reclaimed until the garbage collector deems it necessary.  People 
>>> often mistake that for a memory leak when it is in fact the garbage 
>>> collector doing its work (or rather, doing as little work as it can 
>>> get away with.) 
>>>
>>> I don't see an actual memory leak in the code you posted but creating 
>>> a new ObjectTemplate for every query isn't very efficient.  If you 
>>> don't use ObjectTemplate features, replace it with Object::New(). 
>>>
>>

-- 
-- 
v8-users mailing list
[email protected]
http://groups.google.com/group/v8-users
--- 
You received this message because you are subscribed to the Google Groups 
"v8-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to