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

Use move in variant_deserializer #3734

Closed
danielmewes opened this issue Feb 5, 2015 · 3 comments
Closed

Use move in variant_deserializer #3734

danielmewes opened this issue Feb 5, 2015 · 3 comments
Assignees
Milestone

Comments

@danielmewes
Copy link
Member

The current implementation:

template <cluster_version_t W, int N, class Variant, class T, class... Ts>
struct variant_deserializer<W, N, Variant, T, Ts...> {
    static MUST_USE archive_result_t deserialize_variant(int n, read_stream_t *s, Variant *x) {
        if (n == N) {
            T v;
            archive_result_t res = deserialize<W>(s, &v);
            if (bad(res)) { return res; }
            *x = v;

            return archive_result_t::SUCCESS;
        } else {
            return variant_deserializer<W, N + 1, Variant, Ts...>::deserialize_variant(n, s, x);
        }
    }
};

We should make this use

*x = std::move(v);

to avoid a potentially expensive copy of the variant (some variants can be vectors for example).

@danielmewes danielmewes added this to the 2.0-polish milestone Feb 5, 2015
@danielmewes
Copy link
Member Author

(I'm not sure if std::move works with boost variants like that, but we should try to find something to the same effect)

@danielmewes danielmewes modified the milestones: subsequent, 2.0-polish Feb 5, 2015
@mlucy
Copy link
Member

mlucy commented Feb 5, 2015

I think some of the boost versions we compile with don't support move semantics. We could instead do *x = T(); deserialize<W>(s, boost::get<T>(x)) or something.

@danielmewes danielmewes modified the milestones: 2.1, subsequent, 2.1-polish Mar 23, 2015
@danielmewes danielmewes modified the milestones: 2.1.x, 2.2-polish Aug 19, 2015
@danielmewes danielmewes self-assigned this Aug 19, 2015
@danielmewes danielmewes modified the milestones: 2.2, 2.1.x Aug 19, 2015
@danielmewes
Copy link
Member Author

Fixed in next 0911906

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

No branches or pull requests

2 participants