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

try to integrate new json_print.c with existing printbuf _to_string functions #132

Open
rgerhards opened this issue Dec 1, 2016 · 2 comments

Comments

@rgerhards
Copy link
Member

rgerhards commented Dec 1, 2016

They are very similar and can be merged easily from a theroretical point of view. Unfortunately, the performance of string generation is a central requirement for libfastjson (as it linearly affects rsyslog performance) and this adds additional challenges for the integration work. A lot of algorithm engineering has gone into the printbuf style of existing functions.

This tracker shall provide background of if and how the integration can be done. If impossible, it should serve as a reference of why the similar code bases are treated differently.

Also, see #120 for background info.

@rgerhards
Copy link
Member Author

rgerhards commented Dec 1, 2016

First testing results: I have used the code from #130, which mimics a large part of the typical rsyslog use case.

We get a notable performance loss when using the new functions vs. the old ones. If I use printbuf_memappend_no_nul as write for the new ones, I can see that printbuf_memappend_no_nul is called more than twice as often as with the old code, and this makes up for the a large part of the runtime difference. So we should focus on why more calls are needed and check if we can reduce them.

But it's not just this. We also see performance losses at other places, e.g. when dumping int it now is first written to a temporary buffer, which needs to be copied later on. I guess that's inevitable for the generic approach. Probably this means we need to keep two different implementations, with one optimized for the string generation case.

@EmielBruijntjes @davidelang and all others: Any suggestion would be welcome.

@rgerhards
Copy link
Member Author

I tried a couple of things, including

  • reducing parameter pushing overhead by defining a param block
  • optimizing call parameter attributes (const, restrict)
  • optimizing printbuf_memappend_no_null

The bottom line always is that the new code is at least 10% slower than the old one. A main reason is possibly the function interface, which now requires function calls via pointers and also has a return value. I also found out that we get another nice 10% performance saving by inlining printfbuf_memappend_no_nul. However, this applies only for the old code, which means the difference becomes even bigger.

Summing it up, the new interface takes roughly 25% to 30% more time for my important use case. Thus I conclude that we cannot replace the old printbuf interface and should probably better look at ways to share as much code as possible via the two subsystems.

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

1 participant