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

Add convenience methods to ShadowRealm #1573

Closed
gentledepp opened this issue Jul 3, 2023 · 3 comments
Closed

Add convenience methods to ShadowRealm #1573

gentledepp opened this issue Jul 3, 2023 · 3 comments

Comments

@gentledepp
Copy link
Contributor

#1406

I am trying to implement the "SetValue" convenience methods to ShadowRealm and add tests for them

gentledepp added a commit to gentledepp/jint that referenced this issue Jul 3, 2023
gentledepp added a commit to gentledepp/jint that referenced this issue Aug 22, 2023
gentledepp added a commit to gentledepp/jint that referenced this issue Aug 22, 2023
@gentledepp
Copy link
Contributor Author

gentledepp commented Aug 22, 2023

So after understanding how a ShadowRealm should work (sorry for not having known earlier)

  • I implemented a convenience overload to evaluate an Esprima.Ast.Script in a shadow realm
  • added some tests (am I missing something here?)
  • and created benchmarks

Unfortunately, the benchmarks do not seem as promising as I hoped they would:
dotnet run -c Release --allCategories ShadowRealm

|                         Method |       Mean |     Error |    StdDev |   Gen0 |   Gen1 | Allocated |
|------------------------------- |-----------:|----------:|----------:|-------:|-------:|----------:|
|                  ReusingEngine | 3,726.3 ns |  74.29 ns | 148.36 ns | 0.5417 | 0.0038 |   3.34 KB |
|              NewEngineInstance | 9,780.1 ns | 194.17 ns | 307.98 ns | 2.7618 | 0.1373 |  16.93 KB |
|                    ShadowRealm | 8,251.2 ns | 164.89 ns | 161.94 ns | 2.2888 | 0.1526 |  14.06 KB |
|     ReusingEngine_ParsedScript |   861.1 ns |  16.68 ns |  25.47 ns | 0.3204 | 0.0019 |   1.97 KB |
| NewEngineInstance_ParsedScript | 5,446.2 ns | 105.62 ns | 151.48 ns | 2.5253 | 0.1297 |  15.51 KB |
|       ShadowRealm_ParsedScript | 4,829.8 ns |  96.42 ns | 219.60 ns | 2.0599 | 0.1373 |  12.64 KB |

The fastest way is, obviously, re-using a single (static) Jint.Engine- even better, when reusing the already parsed Esprima.Ast.Script.

It seems the ShadowRealm is a tiny bit more memory efficient and faster than newing up a new Jint.Engine for every call.
However, I hoped it would be way faster. :-|

Any thoughts?

@gentledepp gentledepp changed the title Add convenienge methods to ShadowRealm Add convenience methods to ShadowRealm Aug 22, 2023
@lahma
Copy link
Collaborator

lahma commented Aug 23, 2023

Most of the construction cost probably comes from global object building. Might need a re-read of the spec to see what can be avoided.

gentledepp added a commit to gentledepp/jint that referenced this issue Aug 28, 2023
gentledepp added a commit to gentledepp/jint that referenced this issue Oct 13, 2023
lahma pushed a commit to gentledepp/jint that referenced this issue Oct 16, 2023
@lahma
Copy link
Collaborator

lahma commented Oct 16, 2023

First iteration merged!

@lahma lahma closed this as completed Oct 16, 2023
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

No branches or pull requests

2 participants