Skip to content

Conversation

@jf-tech
Copy link
Owner

@jf-tech jf-tech commented Sep 30, 2020

Just realized a problem, currently we use a loading cache for goja.Runtime caching keyed by ctx. However that means the caching is per transform. We had scenarios in the past where a service handles many concurrent requests, each of which contains a tiny payload that needs to be parsed and transformed. In that situation, the goja.Runtime caching is completely ineffective.

Instead, we want to share goja.Runtime across different transforms, as long as javascript() are not called at the same time. Thus the introduction of resource pool. We will adaptively create a number of goja.Runtime in the resource pool, and each of the transforms, concurrent or not, requests a runtime from the pool. The hope is the resource pool eventually grow to certain size that any concurrent requests would be satisfied. The pool is also configured to have a minimum idling runtimes, so that when eviction kicks in, it won't completely clear up the cache.

Extensive benchmarks created and verified the new design. With this, now javascript is truly performant enough to fully replace eval.

[update] - given my simple use case of the resource pool: no max limit, no blocking, etc. there is simply no reason to use this full featured resource pool - instead use the much simpler and more performant sync.Pool. And benchmark proved it's much faster and now javascript is nearly on par with eval; and concurrent performance is much better than the full-fledged resource pool usage.

jf-tech added 2 commits September 30, 2020 17:56
Just realized a problem, currently we use a loading cache for goja.Runtime caching keyed by ctx.
However that means the caching is per transform. We had scenarios in the past where a service handles
many concurrent requests, each of which contains a tiny payload that needs to be parsed and transformed.
In that situation, the goja.Runtime caching is completely ineffective.

Instead, we want to share goja.Runtime across different transforms, as long as javascripts are not called
at the same time. Thus the introduction of resource pool. We will adaptively create a number of
goja.Runtime in the resource pool, and each of the transforms, concurrent or not, requests a runtime from
the pool. We don't want to block, so if there is no available runtime in the pool, while the pool will
create a new runtime, it will immediately fail and return. The new execProgram will fallback and create
a new runtime. The hope is the resource pool eventually grow to certain size that any concurrent requests
would be satisfied.

Extensive benchmarks created and verified the new design. With this, now javascript is truly performant
enough to fully replace eval.
@codecov
Copy link

codecov bot commented Sep 30, 2020

Codecov Report

Merging #70 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #70   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           29        29           
  Lines         1257      1266    +9     
=========================================
+ Hits          1257      1266    +9     
Impacted Files Coverage Δ
customfuncs/customFuncs.go 100.00% <100.00%> (ø)
customfuncs/javascript.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6743a7...33caf5d. Read the comment docs.

@jf-tech jf-tech changed the title Switch JS runtime caching to a resource pool DO NOT REVIEW YET - Switch JS runtime caching to a resource pool Sep 30, 2020
@jf-tech jf-tech changed the title DO NOT REVIEW YET - Switch JS runtime caching to a resource pool Switch JS runtime caching to sync.Pool Sep 30, 2020
Copy link
Collaborator

@liangxibing liangxibing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jf-tech jf-tech merged commit 65110e8 into master Sep 30, 2020
@jf-tech jf-tech deleted the jsworkerpool branch September 30, 2020 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants