AST API has inconsistencies about esprima-dotnet HOT 7 CLOSED

If there are no objections to the raised issues, I can try (time permitting) to tackle this and submit a PR.

from esprima-dotnet.

@lahma will probably care about perf impact.

from esprima-dotnet.

From Jint's viewpoint API with concrete types and fields instead of properties is optimal. It fights against API best practices but gives the best performance.

No JIT work for properties (might or might not get inlined) and fast for loops against concrete List without interface dispatch and enumerator allocations.

I'd like to see this considered, maybe in form of checking current BDN benchmarks for effects on changes. Of course Jint benchmarks give the best picture as it needs to go through AST as fast as possible.

from esprima-dotnet.

@lahma We can keep fields if property access is a performance concern but properties have the benefit that we can keep the structure read-write internally (with internal setters) to help with the AST construction phase. Again, if you look at the table, there are very few cases and some members are already properties so I don't expect there to be a huge impact.

No JIT work for properties (might or might not get inlined)

Practically all properties will be dumb and non-virtual accesses so I can't imagine why JIT would shy from inlining those.

and fast for loops against concrete List without interface dispatch and enumerator allocations.

This won't happen if you introduce a List<> that's native to the AST namespace. In fact, you can save on allocations by even making it a struct since it's just a wrapper around an array. You'll keep all the same benefits of BCL's List<> while maintaining a clean and encapsulated API.

I don't know much about how Jint is implemented but my guess would be that immutable structures could pave the way to more global optimizations eventually (e.g. sharing nodes and their optimizations across AST roots and scripts). Anyway, I might be getting ahead of myself and just speculating but my point is that there more than one way to skin the performance cat.

from esprima-dotnet.

Release profiling has showed that JIT can skip dummy getters too, probably related to how much it allows itself to take time. So no need for JIT of course means not only faster startup but also possibly more resources available for JITting other stuff. But of course probably not the biggest thing like you said. INode being an interface brings some baggage too.

Custom struct list is a interesting idea indeed. So internal mutators and public gets I believe, sounds nice. Jint has its own internal Dictionary already based on corefxlab's DictionarySlim, haven't played with the idea of making it struct though... A thing for my todo list!

from esprima-dotnet.

I think resolution of this issue has finally been completed with the recent changes.

from esprima-dotnet.

Closing as done, we can open more specific issues if there's still work to be done. Thanks to all involved!

from esprima-dotnet.