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

Encoding runs in worst-case quadratic time #45

Open
appgurueu opened this issue Dec 15, 2022 · 4 comments · May be fixed by #46
Open

Encoding runs in worst-case quadratic time #45

appgurueu opened this issue Dec 15, 2022 · 4 comments · May be fixed by #46

Comments

@appgurueu
Copy link

Consider the following benchmark which encodes the deeply nested structure [1000, [999, [998, [...]]]]:

local json = require"json"
for _ = 1, 1e3 do
	local t = {}
	for i = 1, 1e3 do
		t = {i, t}
	end
	json.encode(t)
end

this should run in a fraction of a second. Instead, it takes ~2 seconds on my device. This is because string concatenations / recursive table.concat are internally used to encode the JSON:

return "[" .. table.concat(res, ",") .. "]"

This means that the outer loop will have to add the [ and ] brackets in time linear in table.concat(res, ","), which in our example will be just marginally shorter, which in turn will only differ in length by a constant. Thus you do a linear time operation linearly often, resulting in worst-case quadratic encoding runtime in the depth of the nested structure.

The proper fix is to use a shared rope and have all recursive calls write to the same rope. You then only concatenate strings once at the end. Building the string then runs in linear time. You can find an example of this approach in my JSON implementation.

@flamendless
Copy link

Perhaps you could make a PR implementing that solution :)

@Vurv78
Copy link

Vurv78 commented Jun 29, 2023

If you want a fast implementation: https://github.com/Vurv78/qjson.lua

Runs thousands of times faster than this repo

@appgurueu
Copy link
Author

If you want a fast implementation: https://github.com/Vurv78/qjson.lua

Runs thousands of times faster than this repo

Refer to this comment by me.

TL;DR: Your JSON parser is broken and has a time complexity issue as well. Your performance claims in their current form can't be taken seriously.

@Vurv78
Copy link

Vurv78 commented Jun 30, 2023

If you want a fast implementation: https://github.com/Vurv78/qjson.lua
Runs thousands of times faster than this repo

Refer to this comment by me.

TL;DR: Your JSON parser is broken and has a time complexity issue as well. Your performance claims in their current form can't be taken seriously.

Both of these have been addressed. Thanks for bringing it to my attention (also see my other comment)

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