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

Optimize encoding performance in Erlang #150

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gasparch
Copy link

Right now jiffy requires keys in maps or keyword tuples to be strings to be successfully encoded.

If existing keys in map are integers - whole map should be recreated with keys converted to integers - which is huge overhead in terms of CPU/RAM usage.

This patch allows integer to be serialized into JSON as map's key as well.

May be it should be enabled only when some option is passed to encoder.

@gasparch
Copy link
Author

gasparch commented Nov 6, 2017

@davisp ping :) would you consider including this optimization?

Idea is to avoid expensive operation of changing

%{
  1 => "val",
  2 => "val"
}

to

%{
  "1" => "val",
  "2" => "val"
}

before passing it to jiffy encoder.

😈 Bad excuse is that JS JSON.stringify allows it :) :P :P

If yes - I'll rebase it on current master and also update docs.

@davisp
Copy link
Owner

davisp commented Nov 6, 2017

Python allows it as well so I'm not against it.

c_src/encoder.c Outdated
goto done;
}
}
else {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: This "else {" should be moved up to the previous line.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thanks, I'll tidy up and resubmit PR

@gasparch
Copy link
Author

Ok, did small fixes.

Do you want another branch + everything squashed in one commit?

lixen added a commit to lixen/jiffy that referenced this pull request Sep 18, 2018
Optimize encoding performance in Erlang davisp#150
@lixen
Copy link

lixen commented Sep 18, 2018

Just tried this and for me it fails for
jiffy:encode(#{2 => 2, 1 => 1}).

** exception throw: {error,{invalid_object_member_key,1}}
in function jiffy:encode/2 (/Users/mikael/work/infranet-erl/_build/default/lib/jiffy/src/jiffy.erl, line 99)

@gasparch
Copy link
Author

jiffy:encode(#{2 => 2, 1 => 1}).

I'm not sure you are using version from this branch. I think it is not merged yet into main tree :(

@karlseguin
Copy link

Would be nice to merge this

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

Successfully merging this pull request may close these issues.

4 participants