Replace dksjon

[astoff]

Some profiling showed that the language server may spend around 50% of the CPU time inside dkjson's encode routines, and take around 20 ms to encode a large completion response.

We should replace dkjson with a C library, but currently cjson doesn't work reliably in Lua 5.3, see this issue. So we need to wait for a fix or find another native json library.

[elliott-wen]

I confirmed that by replacing dkjson with cjson, we can get a much smoother experience.

By the way, do you think it would be better if we bundle a lua interpreter with digestif so that users can skip the configuration hassle?

In my setting, I compiled lpeg + cjson + lua together. The binary is rather small like 250Kb in Ubuntu.

[astoff]

I confirmed that by replacing dkjson with cjson, we can get a much smoother experience.

Did you find this is the case overall, or just for specific LSP requests like completions with lots of candidates? The latter case can be solved more simply, by limiting to 100 items or so.

That said, if you managed to compile cjson and found it to work reliably in the webasm environment, it's easy to make digestif use it when available and fall back to dkjson otherwise. If this works well for you I'll make it so.

By the way, do you think it would be better if we bundle a lua interpreter with digestif so that users can skip the configuration hassle?

I considered this at some point, but now I think the self-installing script, which runs on LuaTeX's own interpreter, is even easier – for the user and also for us, since the distribution remains portable across OSs. Do you see any additional advantage of the bundled stuff?

[elliott-wen]

Yes. I managed to compile cjson with lua5.3 and replaced dkjson with cjson. Somehow the process was quite smooth to me. Just pulled some c files from cjson git repo and compiled them with my lua interpreter.

On Thu, 9 Jul 2020, 6:07 am Augusto Stoffel, notifications@github.com wrote:

I confirmed that by replacing dkjson with cjson, we can get a much smoother experience.

Did you find this is the case overall, or just for specific LSP requests like completions with lots of candidates? The latter case can be solved more simply, by limiting to 100 items or so.

That said, if you managed to compile cjson and found it to work reliably in the webasm environment, it's easy to make digestif use it when available and fall back to dkjson otherwise. If this works well for you I'll make it so.

By the way, do you think it would be better if we bundle a lua interpreter with digestif so that users can skip the configuration hassle?

I considered this at some point, but now I think the self-installing script https://github.com/astoff/digestif#installation-and-set-up is even easier (for the user and also for us, since the distribution remains portable across OSs). Do you see any additional advantage of the bundled stuff?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/astoff/digestif/issues/11#issuecomment-655673564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLPC3BMS4K23NS7DD6RCF3R2SYXDANCNFSM4J4JYKJQ .

[elliott-wen]

I didn't do a in-depth profiling. But the CPU usage seems to go down a bit. I can come back to you with more details later.

On Thu, 9 Jul 2020, 6:07 am Augusto Stoffel, notifications@github.com wrote:

I confirmed that by replacing dkjson with cjson, we can get a much smoother experience.

Did you find this is the case overall, or just for specific LSP requests like completions with lots of candidates? The latter case can be solved more simply, by limiting to 100 items or so.

That said, if you managed to compile cjson and found it to work reliably in the webasm environment, it's easy to make digestif use it when available and fall back to dkjson otherwise. If this works well for you I'll make it so.

By the way, do you think it would be better if we bundle a lua interpreter with digestif so that users can skip the configuration hassle?

I considered this at some point, but now I think the self-installing script https://github.com/astoff/digestif#installation-and-set-up is even easier (for the user and also for us, since the distribution remains portable across OSs). Do you see any additional advantage of the bundled stuff?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/astoff/digestif/issues/11#issuecomment-655673564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLPC3BMS4K23NS7DD6RCF3R2SYXDANCNFSM4J4JYKJQ .

[quicquid]

@elliott-wen would you be willing to share your patch? It seems dkjson is (marked as) not compatible with Lua 5.4 (see the rock description) and prevents digestif from being built on my machine (Fedora 33, see below). Another reason to replace dkjson might be that there hasn't been an update to the library in five years - I'm not sure it is maintained at all anymore.

$ luarocks install dkjson 2.5 --local --check-lua-versions
dkjson not found for Lua 5.4.
Checking if available for other Lua versions...
Checking for Lua 5.1...
Checking for Lua 5.2...
Checking for Lua 5.3...

Error: No results matching query were found for Lua 5.4.
dkjson 2.5 supports only Lua 5.1 and Lua 5.2 and Lua 5.3 but not Lua 5.4.

[astoff]

@quicquid If you just want a workaround for this problem, you can probably edit the dkjson rockspec and install it. Or use the self-installing wrapper mentioned in the README.

[quicquid]

@astoff thank you, I will givethis a try!

[astoff]

Ok, since dkjson was the only dependency external to texlua, I decided to replace it with my own json implementation. It's also a bit faster than dkjson since it doesn't need to be compatible with old versions of LPeg. I have also decided that Digestif will never depend on cjson, in part because it does not look so well maintained.

I also made it so that Digestif checks if cjson is present at runtime and uses it if that's the case. However, I couldn't find any way to compile cjson and test this. @elliott-wen If you get the chance to test this, please let me know.