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

Export deserialize_json at the C++ level in addition to R #80

Closed
eddelbuettel opened this issue Jan 20, 2023 · 8 comments
Closed

Export deserialize_json at the C++ level in addition to R #80

eddelbuettel opened this issue Jan 20, 2023 · 8 comments

Comments

@eddelbuettel
Copy link
Owner

As seen in #79 there is interest in hitting the deserialize_json() workhorse function directly. (It was also seen that exporting it at the R level brings neglible gains while adding a documentation / test / maintenance burden -- fparse() is still the recommended venue.)

However we can make it available at the C level and the (currently new) branch feature/export_c_level does this. To use it, please

  • add a LinkingTo: directive in the client package, and include the header RcppSimdJson.h to access _deserialize_json()
  • add an Imports: and import any R symbol (to force instantiation so that R becomes aware of the C-callable symbol.

That should do the trick. @shikokuchuo let us know if this works for you. We could help with a more explicit example if need be.

@shikokuchuo
Copy link

Thanks for putting this together so quickly! I can confirm that it works fine mechanically. I will update you on the benefits once I get a chance to test it next week.

@eddelbuettel
Copy link
Owner Author

Excellent. It's a good mechanism we all should probably use more, at least for some things.

My recent experience is quite good with another package pair of mine. So I probably roll this out here for RcppSimdJson, and this would be no change for you as you just need a minimum version larger than current CRAN. So I may bump this to CRAN anyway. But no rush, I can (and maybe should) also wait for some first actual feedback from you. It may be a nice boost for repeated calls.

@eddelbuettel
Copy link
Owner Author

Closed by #80

@shikokuchuo
Copy link

Wow you were quick to get this to CRAN! Luckily I find that the LinkingTo solution works as expected and I can make it ever so slightly faster than simply exporting .deserialize_json(). In terms of what I'm experiencing, compared to fparse() , reading JSON is now c. 8% faster on a typical payload. But in terms of high frequency updates of c. 300-500 bytes each the run time is reduced to 1/5 (<4 microseconds!).

@eddelbuettel
Copy link
Owner Author

I was on a roll yesterday and cleaned a few things up, including an upstream update and a quick detail check with @lemire.

Are you using it from ichimoku on 'frequent small parses'? Then the direct, C-level call will help on both fronts. Maintaining the normal interface is still better for more standard usage patterns, so all good.

@shikokuchuo
Copy link

I am reading from websockets, not an ideal setup but out of my control! Not ichimoku, but as it's all nicely on CRAN now, I will update that package to use the new mechanism. Thanks again!

@shikokuchuo
Copy link

FYI this is the commit that implements this for package 'ichimoku': shikokuchuo/ichimoku@702025

Hope this is a helpful reference in case others want to do the same.

Of course the real power comes from being able to call it from C code as you rightly identify. But that's for another day / something else...

@eddelbuettel
Copy link
Owner Author

eddelbuettel commented Jan 22, 2023

Small world. I did the same for xts (in the older, more mechanical way; for RcppSimdJson it is basically automated thanks to Rcpp) so good to see it pop up too :)

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