Pooling support for high request rates?

[jgimness]

Would there be problems having multiple instances? Example use: A high traffic site that needs to do lots of vue/react SSR. A single nodejs process could become CPU-limited.

The readme says to avoid it, but are there any other consequences?

Try to avoid creating multiple INodeJSService instances since by default, each instance spawns a NodeJS process.

Also, if you are using the static API from multiple threads and the NodeJS process is performing invocations for other threads, you might get unexpected results.

For the static API, does that mean we need to implement our own concurrency control on top of it?

There's an older issue in the aspnet repo where someone put up some sample pooling code, for reference:
aspnet/JavaScriptServices#1442 (comment)

I think it would be so valuable to the .net community to have an out-of-the-box pooled nodejs service.

[JeremyTCD]

Good question, there's technically nothing wrong with multiple Node.js processes. A pool of INodeJSService instances would work fine. My concern was more with users repeatedly creating new instances, the ReadMe could be better worded.

What are your thoughts on Node.js workers for parallelization of CPU intensive tasks? It's not out-of-the-box, but it'll have a much smaller memory footprint than multi-processing.

For the static API, does that mean we need to implement our own concurrency control on top of it?

The static API is thread safe except for Configure:

Any existing NodeJS process is killed and a new one is created in the first javascript invocation after every StaticNodeJSService.Configure call. ... Also, if you are using the static API from multiple threads and the NodeJS process is performing invocations for other threads, you might get unexpected results.

Basically, calling Configure restarts the underlying Node.js process (to apply the new options), so if multiple threads are using the underlying Node.js process when Configure is called, you'll likely get unexpected results.

[jgimness]

Thanks, I'll investigate Node.js workers and let you know what I find out.

I was going off research done by other projects, which seem to prefer separate processes (for isolation and redundancy), like here: https://medium.com/airbnb-engineering/operationalizing-node-js-for-server-side-rendering-c5ba718acfc9

Also ReactJS.NET does a similar model of pooling JS Engines (non-NodeJS, unfortunately) https://github.com/reactjs/React.NET/blob/3a8573a439be231cdd4cd702a6f4f425791b6e25/src/React.Core/JavaScriptEngineFactory.cs#L84

[JeremyTCD]

I've decided to add out-of-the-box parallelization #52 (work in progress). When it's done you'll just need to set OutOfProcessNodeJSServiceOptions.Parallelization to Parallelization.MultiProcessing or Parallelization.MultiThreading.

I think you may be right about separate processes being preferable - it seems there can be significant overhead passing data between threads using workers. We'll see once both have been implemented.

Initial benchmarks for multi-processing are excellent:

[Benchmark]
public async Task<DummyResult[]> INodeJSService_InvokeCpuBound_Multithreaded()
{
    const string dummyModule = @"module.exports = (callback, resultString) => {
// Block CPU
var end = new Date().getTime() + 1000;
while(new Date().getTime() < end){ /* do nothing */ } 

callback(null, {result: resultString});
}";
    const string dummyIdentifier = "dummyIdentifier";
    const string dummyResultString = "success";

    // Act
    const int numTasks = 5;
    var results = new Task<DummyResult>[numTasks];
    for (int i = 0; i < 5; i++)
    {
        results[i] = _nodeJSService.InvokeFromStringAsync<DummyResult>(dummyModule, dummyIdentifier, args: new[] { dummyResultString });
    }

    return await Task.WhenAll(results);
}

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
INodeJSService_InvokeCpuBound_MultiProcessing	1.000 s	0.0002 s	0.0002 s	-	-	-	63.32 KB

[JeremyTCD]

@jgimness #52 adds out-of-the-box multi-process concurrency. Documentation here. Also cleaned up ReadMe issue you brought up. Kindly closet this issue if the solution works for you.

[DaniilSokolyuk]

@JeremyTCD i tried concurency mode now and TryInvokeFromCacheAsync always false, i call that method about 1000 times and its return false always :(
code https://github.com/DaniilSokolyuk/NodeReact.NET/blob/9f9143f79642dc6e5c7829d439ee51abd5c31933/NodeReact/NodeInvocationService.cs#L68

[DaniilSokolyuk]

Ok, I see problem )
Maybe create method T InvokeFromCache(... Func<Stream> streamFacotry) ?

[JeremyTCD]

@DaniilSokolyuk Oh yeah good catch, at present you end up caching in the "next" Node.js process in the pool. I like the factory suggestion, would mean users don't have to test for success too. New overloads:

If module isn't cached in current Node.js process, factory is invoked and result is cached:

Task<T> InvokeFromCacheAsync<T>(string moduleCacheIdentifier, Func<Stream> moduleFactory, 
string exportName = null, object[] args = null, CancellationToken cancellationToken = default)

Task<T> InvokeFromCacheAsync<T>(string moduleCacheIdentifier, Func<string> moduleFactory, 
string exportName = null, object[] args = null, CancellationToken cancellationToken = default)

At the same time I'm also thinking of adding overloads so args can be a params argument, been wanting to do this for awhile. E.g:

Task<T> InvokeFromCacheAsync<T>(string moduleCacheIdentifier, Func<string> moduleFactory, 
params object[] args = null)

Thoughts on these?

Edit: Removed redundant overloads I suggested.

[JeremyTCD]

@DaniilSokolyuk Fix underway here: #57. Let me know if you have any input on the expanded API.

[JeremyTCD]

Out-of-the-box concurrency documentation.