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

Is there a way to turn the AST back into a graphql query string? #11

Closed
mcintyre321 opened this issue Apr 6, 2017 · 14 comments · Fixed by #149
Closed

Is there a way to turn the AST back into a graphql query string? #11

mcintyre321 opened this issue Apr 6, 2017 · 14 comments · Fixed by #149
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@mcintyre321
Copy link

I'm interested into urning Linq Expressions into graphql queries (e.g. so a .NET client can treat a graphQL endpoint as an IQueryable), and was thinking I could use your AST to do this, but I can't find a way to turn it back into a graphql query string. Is there a way to do this?

@joemcbride
Copy link
Member

There is an AstPrinter in the Core project, though there isn't one for this parser. For historical reasons, the AST in this project is slightly different than the Core project. The Core project currently does a conversion from this AST to the AST it understands.

@listepo-alterpost
Copy link

@joemcbride way not use original parser libgraphqlparser ?

@joemcbride
Copy link
Member

@listepo-alterpost I'm not a C++ developer and frankly I'm not sure I would want to support that myself. If you think you can get it working I would be interested in seeing it.

@deinok
Copy link
Member

deinok commented Mar 21, 2018

@joemcbride and @listepo-alterpost It would be nice to use that, so we can port or link that lib and let the C++ developers of graphQL build the parser while we only add the links with .NET

@mkmarek
Copy link
Member

mkmarek commented Sep 24, 2018

Hi, might be a bit overdue, but recently I needed to turn GraphQL AST back to query as well so I implemented a printer class doing exactly that.

PR here: #20

@joemcbride Could you have a peek if there is something you'd like to add or remove for the sake of https://github.com/graphql-dotnet/graphql-dotnet?

@deinok
Copy link
Member

deinok commented Sep 24, 2018

@joemcbride and @listepo-alterpost about https://github.com/graphql/libgraphqlparser

There are some issues I have found:

PROS:

  • Faster, really much faster.
  • Official Implementation

CONS:

  • Can only compiled in Linux or MacOS
  • Requirement to generate the library for diferent RID (linux-arm, linux-x64, win-x86, win-x64, etc...)

My personal opinion:
Seems like we have a working implementation that doesn't require anything native, meaning that we don't care about the arch.
In order to be as standard as posible, just make an exact port of https://github.com/graphql/graphql-js/tree/master/src/language but in .NET so we can be official as possible

Edit:
We can make an experiment about how much the performace win is when using the C library

@mcintyre321
Copy link
Author

Keeping a port up to date is an ongoing maintenance burden, and can lead to the death of a project ("sorry we aren't on the latest version").

Could the js version be called from .NET using JINT or similar?

@mcintyre321
Copy link
Author

mcintyre321 commented Sep 25, 2018

OTOH if there is a working implementation from @mkmarek , then we should just run with it. Might be worth making it a pluggable type so that other implementations could be used.

@deinok
Copy link
Member

deinok commented Sep 25, 2018

Using JINT will cause some performace penalizations.
My impression is that part of the library is pretty stable, so tracking changes should be easy. I would like to ask for better split of the flow types so its more easy to track changes

@DSilence
Copy link

DSilence commented Oct 9, 2018

I've tested a native adapter vs managed implementation in my fork (https://github.com/DSilence/parser).
Looks like managed implementation is significantly faster on .net core 2.1.

BenchmarkDotNet=v0.11.1, OS=Windows 10.0.17763
Intel Core i7-7700HQ CPU 2.80GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=2.1.302
  [Host]     : .NET Core 2.1.4 (CoreCLR 4.6.26814.03, CoreFX 4.6.26814.02), 64bit RyuJIT
  DefaultJob : .NET Core 2.1.4 (CoreCLR 4.6.26814.03, CoreFX 4.6.26814.02), 64bit RyuJIT

Method Mean Error StdDev Gen 0 Allocated
Native 46.93 us 0.4059 us 0.3169 us - 32 B
Managed 27.77 us 0.5695 us 1.1633 us 10.3760 32832 B

I've used windows port from this PR graphql/libgraphqlparser#67.
Keep in mind that I also don't construct a managed wrapper around the resulting AST (I only retrieve a ponter to it in the code). This will result in additional slowdowns.
I'm definitely not an expert of native code interop, so it might be that the interop code could be improved significantly.

@deinok
Copy link
Member

deinok commented Oct 9, 2018

your code seems good in interop terms.
Just to check.

ibgraphqlparser is built with CMake. If a sufficiently-recent version of Flex and Bison are installed on your system, it will use them; otherwise, it will rely on the checked-in parser.tab.{c,h}pp and lexer.{h,cpp}.

Do yo compile that with Flex and Bison? Can you run the same benchmark on any Linux? (Just to have more information)

@DSilence
Copy link

DSilence commented Oct 9, 2018

I've used https://github.com/lexxmark/winflexbison, as the PR author suggested. Hopefully I will have some time to try running the whole thing on linux, since windows is not officially supported by libgraphqlparser yet, which may very well be the cause of the slowdowns.
I also have some ideas for managed implementation improvements (using more structs instead of classes, span + slice instead of strings and substrings, not throwing parsing exceptions but storing and gathering them) which I think will be interesting to throw into that competition.

@DSilence
Copy link

I rewrote the managed lexer/parser to utilize span API (at least partially). Below are the results of running the benchmark on Ubuntu 18.04 (I've used Azure VM). I have installed the latest versions of Flex and Bison before running it.

BenchmarkDotNet=v0.11.1, OS=ubuntu 18.04
Intel Xeon CPU E5-2673 v4 2.30GHz, 1 CPU, 2 logical cores and 1 physical core
.NET Core SDK=2.1.403
  [Host]     : .NET Core 2.1.5 (CoreCLR 4.6.26919.02, CoreFX 4.6.26919.02), 64bit RyuJIT
  DefaultJob : .NET Core 2.1.5 (CoreCLR 4.6.26919.02, CoreFX 4.6.26919.02), 64bit RyuJIT

Method Mean Error StdDev Gen 0 Gen 1 Allocated
Native 327.85 us 6.3704 us 7.5835 us - - 2.36 KB
Managed 35.67 us 0.7059 us 0.7249 us 2.8687 0.2441 19.06 KB

@sungam3r
Copy link
Member

sungam3r commented Jan 2, 2022

Will be published in v8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants