-
Notifications
You must be signed in to change notification settings - Fork 241
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
Use new getJsonObject kernel for json_tuple #10635
Use new getJsonObject kernel for json_tuple #10635
Conversation
Signed-off-by: Haoyang Li <[email protected]>
a quick perf test:
6 fields: 1 fields: Wow, so it is actually quite fast. Not sure if I tested it right. |
A bit of feedback on the quick test.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might also be nice to have a follow on issue to see if we can drop the special field name checks.
Signed-off-by: Haoyang Li <[email protected]>
Updated, the special field name checks are safe to drop. |
build |
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
build |
Signed-off-by: Haoyang Li <[email protected]>
build |
Signed-off-by: Haoyang Li <[email protected]>
Verified again to generate doc. Seems that |
build |
Signed-off-by: Haoyang Li <[email protected]>
build |
This PR updates json_tuple with new getJsonObject kernel.
All current xfailed cases got passed:
I think the performance will not be good because it calls getJsonObject kernel many times, which is not very fast by itself.
With the new json_parser in jni, I think we can implement a kernel for json_tuple to get much higher performance by passing all fields in one pass. So this PR will be a short-term workaround, even if it gets merged.
Depends on NVIDIA/spark-rapids-jni#1893