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 tuple_tostring and tuple to yaml encoding #128

Closed
rtsisyk opened this issue Nov 21, 2013 · 2 comments
Closed

Optimize tuple_tostring and tuple to yaml encoding #128

rtsisyk opened this issue Nov 21, 2013 · 2 comments
Assignees
Labels
customer feature A new functionality
Milestone

Comments

@rtsisyk
Copy link
Contributor

rtsisyk commented Nov 21, 2013

Current implementation unpacks entire tuple to Lua and then calls yaml.encode on it.
It was the simplest solution I've found for support MsgPack tuples.

    /*
     * The method does next things:
     * 1. Calls :unpack
     * 2. Serializes the result using yaml
     * 3. Strips start and end of yaml document symbols
     */

    /* unpack */
    lbox_tuple_totable(L);

    /* serialize */
    lua_replace(L, 1);
    yamlL_encode(L);

    /* strip yaml tags */
    size_t len;
    const char *str = lua_tolstring(L, -1, &len);
    assert(strlen(str) == len);
    const char *s = index(str, '[');
    const char *e = rindex(str, ']');
    assert(s != NULL && e != NULL && s + 1 <= e);
    lua_pushlstring(L, s, e - s + 1);
    return 1;

Please use libyaml directly and "remove the PHP" (c) Kostja from the code!

See also #30

@rtsisyk rtsisyk modified the milestones: 1.6.3, 1.6.2 Mar 5, 2014
@kostja kostja modified the milestones: 1.7, 1.6.3 May 22, 2014
@rtsisyk rtsisyk modified the milestones: 1.7.5, wishlist May 24, 2017
@rtsisyk
Copy link
Contributor Author

rtsisyk commented May 24, 2017

Needed for @zlobspb
Solution is to use mp_snprint() function from libmsgpuck.

ilmarkov added a commit that referenced this issue Jun 16, 2017
Change implementation of tuple_to_string

Old variant coverted tuple to lua table, then encoded to yaml
New variant encodes tuple to yaml.
Add new function to API box_tuple_to_string

Relates #128

Results of benchmarking with:

	old: 1m27.555s
	new: 0m50.830s
	Acceleration 43%
ilmarkov added a commit that referenced this issue Jun 19, 2017
Old variant converted tuple to lua table, then encoded to yaml
New variant encodes tuple to yaml.

Relates #128

Results of benchmarking with:

	old: 1m27.555s
	new: 0m48.738s
	Acceleration 44%
rtsisyk added a commit that referenced this issue Jun 20, 2017
* Inline IS_PRINTABLE macros into check_utf8()
* Rename yaml_check_utf8() to utf8_check()
* Get rid of libyaml-private dependency for lua-yaml

Needed for #128
rtsisyk added a commit that referenced this issue Jun 20, 2017
This code was added by me, it is not covered by lua-yaml
copyrights and can be moved to src/.

See cd0d6f2 "incorrect error handling in lyaml".

Needed for #128
rtsisyk added a commit that referenced this issue Jun 20, 2017
No semantic changes.

Needed for #128
rtsisyk pushed a commit that referenced this issue Jun 20, 2017
Old variant converted tuple to Lua table and then encoded to yaml
New variant encodes tuple to yaml directly.

Results of benchmarking with:

	old: 1m27.555s
	new: 0m48.738s
	Acceleration 44%

Closes #128
rtsisyk added a commit that referenced this issue Jun 20, 2017
* Fix handling of encoded data
* Remove \n added by libyaml
* Add a test case

Follow up #128
@rtsisyk
Copy link
Contributor Author

rtsisyk commented Jun 20, 2017

1.7.4-177-g0a7ca4b2e

@rtsisyk rtsisyk closed this as completed Jun 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer feature A new functionality
Projects
None yet
Development

No branches or pull requests

3 participants