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

Using JAQ as an alternative to mustache templates #1321

Closed
tusharmath opened this issue Mar 8, 2024 · 21 comments
Closed

Using JAQ as an alternative to mustache templates #1321

tusharmath opened this issue Mar 8, 2024 · 21 comments

Comments

@tusharmath
Copy link
Contributor

tusharmath commented Mar 8, 2024

./jq is an interesting DSL to perform JSON transformations. Embedding JQ can give developers a lot of flexibility when compared to existing mustache templates. The current mustache templates only support accessing values by path, with JQ we should be able to do much more.

Proposal

 type Post {
    id: Int!
    userId: Int!
    title: String
-   user: User @http(path: "/users/{{value.userId}}")
+   user: User @http(path: "/users/{{.value.userId}}")
 }

Technical Requirements

  • Use https://github.com/01mf02/jaq to implement JQ
  • Write benchmarks comparing existing and new implementations
  • Compile and store JQ templates at compile time (Blueprint generation time)

Design

  • Use a JAQ parser to parse a JAQ template and replace the mustache parser with JAQ
  • Implement an evaluator, that can take any JsonLike object and transform it using JAQ.
@tusharmath
Copy link
Contributor Author

/bounty 600

Copy link

algora-pbc bot commented Mar 8, 2024

💎 $600 bounty created by tailcallhq
🙋 If you start working on this, comment /attempt #1321 along with your implementation plan
👉 To claim this bounty, submit a pull request that includes the text /claim #1321 somewhere in its body

🙏 Thank you for contributing to tailcallhq/tailcall!.
🧐 Checkout our guidelines before you get started.

Attempt Started (GMT+0) Solution
🔴 @ssddOnTop Mar 9, 2024, 6:46:34 AM WIP
🔴 @digital-phoenix Apr 4, 2024, 4:13:27 PM WIP

@ssddOnTop
Copy link
Member

ssddOnTop commented Mar 9, 2024

/attempt 1321

Copy link

algora-pbc bot commented Mar 9, 2024

@ssddOnTop: The Tailcall Inc. team prefers to assign a single contributor to the issue rather than let anyone attempt it right away. We recommend waiting for a confirmation from a member before getting started.

@digital-phoenix
Copy link
Contributor

@tusharmath could you assign this to me?

@tusharmath
Copy link
Contributor Author

Deprioritizing this for now.

@tusharmath tusharmath mentioned this issue Mar 28, 2024
34 tasks
@tusharmath tusharmath reopened this Mar 28, 2024
@digital-phoenix
Copy link
Contributor

@tusharmath can I start working on this?

@tusharmath
Copy link
Contributor Author

Please go ahead

@digital-phoenix
Copy link
Contributor

digital-phoenix commented Apr 4, 2024

/attempt #1321

Algora profile Completed bounties Tech Active attempts Options
@digital-phoenix    1 tailcallhq bounty
+ 5 bounties from 2 projects
Rust, C#,
Python
Cancel attempt

@digital-phoenix
Copy link
Contributor

@tusharmath there is an issue with the way jaq operates and jsonlike's current capabilities. jaq needs to know about the type of data it is given. But jsonlike does not currently have a way of directly supplying this information. The best jsonlike can do is try all of the as_ functions to see which one returns correctly. Unless jsonlike is updated to be able to provide type data there is no practical way of getting jaq to work on jsonlike data.

@digital-phoenix
Copy link
Contributor

@tusharmath can I change the design of the jsonlike trait to support jaq?

@amitksingh1490
Copy link
Collaborator

@digital-phoenix sure you can change the design of jsonlike but try to keep the changes minimal also it should not break other functionalities, like wasm, grpc etc.
I would suggest before implementing propose the changes here or on discord contributors channel.

@digital-phoenix
Copy link
Contributor

@amitksingh1490 the simplest thing to do would be to serialize the jsonlike trait. But that would lose the advantage of lazy evaluation. I'll see if adding a gettype function (a function that returns the current type) to jsonlike is sufficient for working with JAQ.

Copy link

algora-pbc bot commented Apr 5, 2024

@digital-phoenix: Reminder that in 1 days the bounty will become up for grabs, so please submit a pull request before then 🙏

@digital-phoenix
Copy link
Contributor

@amitksingh1490 Unless I'm missing an opportunity to add indirection I think JAQ currently needs object to be fully realized as a map of string value pairs. I could add functionality to JAQ to support lazy evaluation but that's probably beyond the scope of this issue. For now can I add an as_map function to convert a jsonlike trait to a map of key value pairs?

Copy link

algora-pbc bot commented Apr 6, 2024

The bounty is up for grabs! Everyone is welcome to /attempt #1321 🙌

@mudratech
Copy link

@tusharmath can I start working on this?

@tusharmath
Copy link
Contributor Author

Please do.

@avdb13
Copy link

avdb13 commented Apr 18, 2024

Anyone still trying to get this finished? Or are the goals of lazily evaluating types implementing JsonLike impossible to meet after all?

@ssddOnTop
Copy link
Member

ssddOnTop commented May 5, 2024

I performed some benchmarks after implementing a new @jq directive

config used:

schema
  @server(port: 8000)
  @upstream(baseURL: "http://jsonplaceholder.typicode.com") {
  query: Query
}

type Query @cache(maxAge: 60000) {
  posts: [Post] @http(path: "/posts")
  users: [User] @http(path: "/users")
  user(id: Int!): User @http(path: "/users/{{.args.id}}")
}

type User {
  id: Int!
  name: String!
  username: String!
  email: String!
  phone: String
  website: String
}

type Post {
  id: Int!
  userId: Int!
  title: String!
  body: String!
  curId: String! @jq(query: ".id")
}

wrk.lua:

wrk.method = "POST"
wrk.body = '{"operationName":null,"variables":{},"query":"{posts { curId }}"}' -- change curId to id to compare b/w jq impl and currnet impl
wrk.headers["Connection"] = "keep-alive"
wrk.headers["Content-Type"] = "application/json"
wrk.timeout = 10

command:
wrk -d 30 -t 4 -c 100 -s wrk.lua http://localhost:8000/graphql > wrk-output.txt

according to 01mf02/jaq#173 (comment) ValT (a trait) can be used as 0 cost abstraction to avoid serialization b/w serde_json::Value, async_graphql::Value, etc.

so I created a custom wrapper for async_graphql::Value (as it's used in majority of our codebase) as pub struct ConstVal(pub async_graphql::Value) and implemented ValT for it.

I performed benchmarks on:

  1. custom ValT implementation
  2. on async_graphql::Val (aka ConstValue) -> serde_json::Value -> jaq_intersept::Val
  3. on id field to compare benches

here are the benchmarks:

Configuration Req/Sec Range Latency Average Latency Max Requests Data Read Requests/sec Transfer/sec
curId (with custom impl ValT) 2.71k - 3.60k 20.50ms 1.88s 320,849 435.11MB 10,661.01 14.46MB
curId (on ConstVal -> serde_json::Value -> Val) 2.58k - 3.71k 31.16ms 1.99s 301,216 408.49MB 10,011.74 13.58MB
id 10.97k - 16.52k 18.75ms 1.98s 1,288,105 1.35GB 42,861.92 45.86MB
`curId (with custom impl ValT):`
Running 30s test @ http://localhost:8000/graphql
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    20.50ms   89.20ms   1.88s    97.88%
    Req/Sec     2.71k   337.90     3.60k    90.22%
  320849 requests in 30.10s, 435.11MB read
  Socket errors: connect 0, read 0, write 0, timeout 2
Requests/sec:  10661.01
Transfer/sec:     14.46MB

`curId (on ConstVal -> serde_json::Value -> Val):`
Running 30s test @ http://localhost:8000/graphql
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    31.16ms  136.13ms   1.99s    97.03%
    Req/Sec     2.58k   297.56     3.71k    87.49%
  301216 requests in 30.09s, 408.49MB read
  Socket errors: connect 0, read 0, write 0, timeout 7
Requests/sec:  10011.74
Transfer/sec:     13.58MB

`id:`
Running 30s test @ http://localhost:8000/graphql
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    18.75ms  118.55ms   1.98s    97.51%
    Req/Sec    10.97k     1.37k   16.52k    85.06%
  1288105 requests in 30.05s, 1.35GB read
  Socket errors: connect 0, read 0, write 0, timeout 6
Requests/sec:  42861.92
Transfer/sec:     45.86MB

Conclusion:
Req/sec remains almost the same with or without implementing ValT on small data sets. Also to run the query, we need to clone whole context value and hence it consumes a lot more memory. And jq is 4x slower then current implementation.

In my previous pr (#1801) I performed benchmark to see the difference b/w current Mustache implementation and JQ, it turns out that is is 8x slower then mustache.

(All these tests were performed on M1 max macbook)
(All the benchmark data provided is based on sample code done in #1866)

@tusharmath
Copy link
Contributor Author

Thanks @ssddOnTop for providing details about the benchmarks. I think this should close the chapter on JQ. Overall I see two problems:

  • JQ is hard to use, it could have been worth it if it were really fast.
  • JQ implementation is quite slow, unless we implement JQ from scratch, it isn't worth integrating or learning this new syntax.

I will close the JQ integration for now. Instead, we will leverage JS and JS only for doing anything custom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants