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

Returning multiple values #186

Open
printercu opened this issue Jun 6, 2019 · 8 comments
Open

Returning multiple values #186

printercu opened this issue Jun 6, 2019 · 8 comments
Labels
feature A new functionality router teamS Scaling
Milestone

Comments

@printercu
Copy link

vshard's call does not return all values from called procedure:

> vshard.router.routeall()[uuid].master.conn:call('asd')
---
- 1
- 2
...
> vshard.router.call(3000, 'read', 'asd')
---
- 1
...
@Gerold103 Gerold103 added the duplicate This issue or pull request already exists label Jun 7, 2019
@Gerold103
Copy link
Collaborator

Yes, and it is documented. We can't support universal multireturn since it would hit performance to create lots of temporary tables inside the router.
Additionally, it is a duplicate of #49.

@printercu
Copy link
Author

Are this expenses comparable with computations required for performing request? Benchmark shows that the difference between wrapping/unwrapping result with table and returning result as is is less then 1ns. This is not significant comparing to net request timing.

@Gerold103
Copy link
Collaborator

Less than 1 ns? Please, share with me these benches. 1ns is ~3 processor ticks. It is impossible to pack and unpack a Lua table so fast.

@knazarov
Copy link

In addition, the second return value is reserved for error messages. So whenever the user needs to do multireturn, they should do it through creating a table and returning it as first value.

@printercu
Copy link
Author

local clock = require('clock')

local function source(x)
    return x + 1, x + 2
end

local function with_wrap(x)
    local result = {source(x)}
    return unpack(result)
end

local function without_wrap(x)
    local result = source(x)
    return result
end

local function measure(n, fn)
    local start = clock.time()
    for i = 1, n, 1 do
        fn(i)
    end
    print(clock.time() - start)
end

local n = 1000000
measure(n, with_wrap)
measure(n, without_wrap)

I get

0.070491075515747
0.00034904479980469

Sorry, I had mistake - it's 69ns per call.

@Gerold103
Copy link
Collaborator

Gerold103 commented Jun 10, 2019

Ok. Firstly, in without_wrap you return only the first value, not all of them. But it is ok, nothing changes really. But most importantly - your source function returns only two integers. It is very artificial example, not representing anything, sorry. It works fast only because LuaJit has small amount of preallocated tiny memory chunks for such cases, and returns them in nanoseconds.

But try to add one more integer - it works twice slower. Add fourth integer - it works x100 times slower (microseconds).
I get these results on my laptop with another source function:

local function source(x)
    return x + 1, x + 2, x + 3, x + 4
end

28.444905042648 - wrap
0.39218592643738 - no wrap

@Gerold103
Copy link
Collaborator

There is a couple of options how to handle multireturn without table packing.

  • Use raw netbox API. Currently vshard router uses netbox:call() to call vshard.storage.call(). It takes not more than 3 result values. This is because of perf problems with wrapping values into a table. But what netbox:call() does internally is that it anyway keeps result in a table, but calls unpack() in return. Like this:
function remote_methods:call(func_name, args, opts)
    check_remote_arg(self, 'call')
    check_call_args(args)
    args = args or {}
    local res = self:_request('call_17', opts, nil, tostring(func_name), args)
    if type(res) ~= 'table' or opts and opts.is_async then
        return res
    end
    return unpack(res)
end

remote_methods:call is implementation of netbox:call(). It is possible to create a new function like netbox_call_raw in vshard's code, which takes the same as remote_methods:call, and does the same except it does not call unpack() in the end. Then the values can be carried around and unpacked right before return from vshard.router.call(). For free. Because We don't pack/unpack anything except when data is read from the socket. Problem of this solution is that we need to use netbox._request not documented function. Which is quite mutable really. Hard to rely on it.

  • Second option - try rewriting vshard.router.call() using recursion with tail call optimization. That would allow to carry ... until it is returned.

@Gerold103 Gerold103 reopened this Mar 27, 2020
@Gerold103 Gerold103 added router feature A new functionality and removed duplicate This issue or pull request already exists labels Mar 29, 2020
@Gerold103 Gerold103 added this to the 0.3 milestone Mar 29, 2020
@Gerold103
Copy link
Collaborator

Another idea - use netbox is_async always. Just manually call wait_result for blocking requests. This is anyway exactly what netbox does internally. is_async returns result as a table without any special packing inside, so it should work. Although that would mean to drop 1.9 out of support entirely.

@kyukhin kyukhin added the teamS Scaling label Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new functionality router teamS Scaling
Projects
None yet
Development

No branches or pull requests

4 participants