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

Feedback: Memory issues running in production #161

Closed
mattwoberts opened this issue Sep 21, 2015 · 13 comments

Comments

Projects
None yet
5 participants
@mattwoberts
Copy link

commented Sep 21, 2015

Hi,

Just feeding back about my experiences using React.NET in a web app I manage. I'm using react code for 2 pages in the app, which are frequently accessed edtitable lists.

The web app gets about 10,000 views a day.

So, using React.NET, I plumbed in the server-side rendering and everything seemed to work great. I load-tested with a simple "request the same react page thousands of times" test, and things seemed OK. So we went live with this (to an azure web site)...

While everything looked good, we noticed that memory usage started creeping up.. Over a period of about 3 days, the memory usage from IIS went from 800Meg to about 2 Gig, where it then maxed out and the app started getting memory exceptions (the server had 3 gig). What seemed to be the issue was that for each different user logged in to the app, this would have an impact to the amount of memory that was then being consumed and got given back (presumably by the ClearScript engine spinning up and processing the JS for that user). So if I just bombarded it with a simple load test for the same user, I didn't see much of a problem.

For now, I've turned off server rendering, and the memory issue has pretty much gone away. I know there is / has been some talk of changing the code so that the ClearScript engine is disposed better and not pooled, I think this needs to move up the list a bit ;) I've not tried to port the app to use the "super charged" fork/version, mainly because of time constraints.

@PeteDuncanson

This comment has been minimized.

Copy link

commented Sep 21, 2015

Hi Matt,

The issue is with your global vars not getting cleared up properly in ClearScript. You could try adding a bit of clean up code that runs at the end of your main entry point and ideally only on the server. We've had a few instances where we need to do just server-side stuff and a quick check it the window object exists seems to do the trick. Something like this might work:

var App = React.createClass({
onComponentUnMount : function() {
// Clear out anything we've put into global
delete MyStore;
delete MyLumpOfGlobalJSON;
},

render: function() {
return

Testing


}
});

If you want to get fancy you can automatically loop over global vars too but for the amount we create I've found it ok to just hard code them and be implicit about it. Remy Sharp has a sample to get you started though, https://remysharp.com/2007/11/01/detect-global-variables

I'm planning on moving some of our mods over to React.net but in the meantime if you want to get up and running with SuperChargedReact its possible although I know the documentation for it is sparse at the minute. It really is just two DLL's, an optional JS file and Snippet of code very similar to React.net

Pete

@Daniel15

This comment has been minimized.

Copy link
Member

commented Sep 21, 2015

Another thing you could try is disabling engine pooling/reuse in the
config. This should not leak any memory, but it is slower as it needs to
spin up a new JavaScript engine every time.

Sent from my phone.
On Sep 21, 2015 5:44 AM, "Pete Duncanson" notifications@github.com wrote:

Hi Matt,

The issue is with your global vars not getting cleared up properly in
ClearScript. You could try adding a bit of clean up code that runs at the
end of your main entry point and ideally only on the server. We've had a
few instances where we need to do just server-side stuff and a quick check
it the window object exists seems to do the trick. Something like this
might work:

var App = React.createClass({
onComponentUnMount : function() {
// Clear out anything we've put into global
delete MyStore;
delete MyLumpOfGlobalJSON;
},

render: function() {
return

Testing

}
});

If you want to get fancy you can automatically loop over global vars too
but for the amount we create I've found it ok to just hard code them and be
implicit about it. Remy Sharp has a sample to get you started though,
https://remysharp.com/2007/11/01/detect-global-variables

I'm planning on moving some of our mods over to React.net but in the
meantime if you want to get up and running with SuperChargedReact its
possible although I know the documentation for it is sparse at the minute.
It really is just two DLL's, an optional JS file and Snippet of code very
similar to React.net

Pete


Reply to this email directly or view it on GitHub
#161 (comment).

@mattwoberts

This comment has been minimized.

Copy link
Author

commented Sep 22, 2015

Cheers guys. Just hope this information is useful to you. I'll try and play with SuperCharged, but time constraints might not allow.

@PeteDuncanson If you do get anywhere with a pull request to bring your changes into this repo, then I'd be happy to help you test it out - just give me a shout

@jslatts

This comment has been minimized.

Copy link
Contributor

commented Sep 23, 2015

I think my app may be subject to the same issue. How are you guys profiling V8 to watch memory growth? I have been trying to use the Visual Studio profiler, but it keeps crashing on me. Is there a better way?

Related questions:

  • With pooling turned on, is Clearscript/V8 isolating the javascript context between requests?
  • Do the engines in the pool get recycled periodically? I was reading through the code but I didn't see any configuration value for this.
@Daniel15

This comment has been minimized.

Copy link
Member

commented Sep 23, 2015

With pooling turned on, is Clearscript/V8 isolating the javascript context between requests?

At the moment it's reusing the same context. For VroomJs (V8 on Linux/Mac) when pooling is turned off, we just use one engine and create a new context for every request. Perhaps we should do the same for ClearScript too. I'm considering dropping JavaScriptEngineSwitcher and having a direct dependency on ClearScript (for Windows) and VroomJs (for Linux and Mac) which should allow V8-specific optimisations.

Do the engines in the pool get recycled periodically?

JSPool has a MaxUsagesPerEngine configuration option, it might not be exposed in ReactJS.NET though :( Feel free to file a task for that if you like, or you can hack it into ReactJS.NET

@jslatts

This comment has been minimized.

Copy link
Contributor

commented Sep 23, 2015

Thanks Daniel, that is helpful. So on the windows side, with pooling off, a new engine is created per request?

This might be more for @PeteDuncanson:

I read through your other issues and the referenced Clearscript issues as well. I just ran an experiment to confirm that I can delete out my global variables if needed, but now I'm wondering if that is actually required...

If the context is shared between all requests, then anything defined globally will only exist once per engine, correct? In my case, this means libraries (like React) and my top level components. When React.createElement() is called, it creates a new instance in the local scope that is used for calling React.renderToString(). Assuming we don't do anything to create variables outside of the local scope, shouldn't these objects be available for GC immediately after the request is finished?

I'm probably missing something here. Just want to make sure I'm not setting myself up for surprises down the road here. We are getting awfully close to shipping :)

@Daniel15

This comment has been minimized.

Copy link
Member

commented Sep 24, 2015

So on the windows side, with pooling off, a new engine is created per request?

That's right. I think once I drop JavaScriptEngineSwitcher I'll make it reuse the same engine and just create a new context per request.

Assuming we don't do anything to create variables outside of the local scope, shouldn't these objects be available for GC immediately after the request is finished

That's right. You only need to delete global variables if you're actually creating global variables per-request. Local variables are fine.

@Daniel15 Daniel15 added the Discussion label Sep 24, 2015

@PeteDuncanson

This comment has been minimized.

Copy link

commented Sep 24, 2015

We've been using https://www.jetbrains.com/dotmemory/ and http://www.red-gate.com/products/dotnet-development/ants-memory-profiler/ for checking memory usage. Ants has better support for showing unmanged memory which is the main issue with ClearScript.

We are a bit spoilt with .net's Garbage Collection but in ClearScript its a bit more lazy, it seems if you set a global var it hangs around, deleting it was the only way to flag that we wanted it gone and at that point GC might take a look at it at its leisure. If you don't manually clear the vars up in global then they site around gobbling memory until some unknown time where they might get cleaned up (or not). Even forcing Garbage Collection on V8 had little effect.

@Daniel15 I think having a way to split between the two (ClearScript and Vjoom) is the way to go. At the minute we have too many abstractions which is making it a chore to do any optimisations. If you can hardcode one or the other then we can tune them to perform at their best I guess? I know nothing about Vjoom though so that one will be all yours :)

@PeteDuncanson

This comment has been minimized.

Copy link

commented Sep 24, 2015

@jslatts Regarding your global scope query, we had some Flux Stores, React Router routes and our JSON data that gets poked into our root component as props all poked into global. That was our issue, if you don't use global you should be good to go and normal JS GC should do its thing. Just be aware the V8 is very lazy at GC and likes to do it sort of at the last minute (ie your memory has to go through the roof until it notices, its not all that good at playing nice with other but thats the price of its blazing speed).

I've got a hazy memory that ClearScript knows if a variable was set from its host too and if so it keeps a handle on it which prevents GC but I need to try to find where I read that to be sure.

@mattwoberts

This comment has been minimized.

Copy link
Author

commented Sep 24, 2015

@jslatts Re: Profiling, I wasn't using any of the profiling tools, since the issue was only apparent when in production - locally I could hit the page thousands of times and not spot much of an issue. For me it was just a case of turning off server rendering and seeing the change in memory usage.

@Daniel15 I think I'm with you and @Daniel15 re: having that split - I also looked at implementing some quick-win optimisations but quickly got lost in the abstractions.

@Daniel15

This comment has been minimized.

Copy link
Member

commented Sep 24, 2015

If I remember correctly, JSPool forces a garbage collection regularly, when
engines are returned to the pool.

Sent from my phone.
On Sep 24, 2015 5:28 AM, "mattwoberts" notifications@github.com wrote:

@jslatts https://github.com/jslatts Re: Profiling, I wasn't using any
of the profiling tools, since the issue was only apparent when in
production - locally I could hit the page thousands of times and not spot
much of an issue. For me it was just a case of turning off server rendering
and seeing the change in memory usage.

@Daniel15 https://github.com/Daniel15 I think I'm with you and @Daniel15
https://github.com/Daniel15 re: having that split - I also looked at
implementing some quick-win optimisations but quickly got lost in the
abstractions.


Reply to this email directly or view it on GitHub
#161 (comment).

@mattwoberts

This comment has been minimized.

Copy link
Author

commented Dec 1, 2015

Just having a look at this after a few months - wondering if anything has changed re: server side rendering and the issue with engine pooling and memory leaks - @PeteDuncanson / @Daniel15 ?

Cheers!

@dustinsoftware

This comment has been minimized.

Copy link
Member

commented Jan 5, 2019

Closing issues older than a year, please re-open if you think this is still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.