[ 
https://issues.apache.org/jira/browse/MESOS-1008?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Benjamin Mahler updated MESOS-1008:
-----------------------------------

    Description: 
The following will discuss {{Try}}, but the same applies to {{Option}} and 
{{Result}}.

Currently retrieving the value from a {{Try}} requires a copy:

{code}
  T get() const { ... return *t; }
{code}

Instead, we can return a const&:

{code}
  const T& get() const { ... return *t; }
{code}

For existing callers, this should be fairly seamless:

{code}
  const T& t = try.get(); // No change needed.
  T t = try.get(); // No change needed, T is already required to be copyable.
  try.get().mutator(); // Will no longer compile, but we should not allow this 
anyway!
{code}

{code}
  const T& t = try.get();
  try = T(); // t is now garbage!
  t.foo(); // No longer works.
{code}

The last case is the most concerning as this mistake cannot be caught at 
compile-time. We could remove the assignment operators, but this seems overly 
restrictive. We could also guard (via {{CHECK}}) the assignment operator after 
a {{get()}} operation, but of course, we're also preventing valid {{Try}} usage 
if we go this route. The best path may simply be to leave the assignment 
operators as is (many callers already make copies).

We can also add C++11 support for move to pilfer the {{Try}}, to avoid copies 
in the caller entirely:

{code}
  T&& move() const { ... } // After a move(), we must guard Try operations.
{code}

  was:
The following will discuss Try, but the same applies to Option and Result.

Currently retrieving the value from a Try requires a copy:

  T get() const { ... return *t; }

Instead, we can return a const&:

  const T& get() const { ... return *t; }

For existing callers, this should be fairly seamless:
  const T& t = try.get(); // No change needed.
  T t = try.get(); // No change needed, T is already required to be copyable.
  try.get().mutator(); // Will no longer compile, but we should not allow this 
anyway!

  const T& t = try.get();
  try = T(); // t is now garbage!
  t.foo(); // No longer works.

The last case is the most concerning as this mistake cannot be caught at 
compile-time. We could remove the assignment operators, but this seems overly 
restrictive. We could also guard (via CHECK) the assignment operator after a 
get() operation, but of course, we're also preventing valid Try usage if we go 
this route. The best path may simply be to leave the assignment operators as is 
(many callers already make copies).

We can also add C++11 support for move to pilfer the Try, to avoid copies in 
the caller entirely:

  T&& move() const { ... } // After a move(), we must guard Try operations.


> Reduce copying in stout / libprocess primitives {Try, Option, Result, Future}.
> ------------------------------------------------------------------------------
>
>                 Key: MESOS-1008
>                 URL: https://issues.apache.org/jira/browse/MESOS-1008
>             Project: Mesos
>          Issue Type: Improvement
>            Reporter: Benjamin Mahler
>            Assignee: Benjamin Mahler
>
> The following will discuss {{Try}}, but the same applies to {{Option}} and 
> {{Result}}.
> Currently retrieving the value from a {{Try}} requires a copy:
> {code}
>   T get() const { ... return *t; }
> {code}
> Instead, we can return a const&:
> {code}
>   const T& get() const { ... return *t; }
> {code}
> For existing callers, this should be fairly seamless:
> {code}
>   const T& t = try.get(); // No change needed.
>   T t = try.get(); // No change needed, T is already required to be copyable.
>   try.get().mutator(); // Will no longer compile, but we should not allow 
> this anyway!
> {code}
> {code}
>   const T& t = try.get();
>   try = T(); // t is now garbage!
>   t.foo(); // No longer works.
> {code}
> The last case is the most concerning as this mistake cannot be caught at 
> compile-time. We could remove the assignment operators, but this seems overly 
> restrictive. We could also guard (via {{CHECK}}) the assignment operator 
> after a {{get()}} operation, but of course, we're also preventing valid 
> {{Try}} usage if we go this route. The best path may simply be to leave the 
> assignment operators as is (many callers already make copies).
> We can also add C++11 support for move to pilfer the {{Try}}, to avoid copies 
> in the caller entirely:
> {code}
>   T&& move() const { ... } // After a move(), we must guard Try operations.
> {code}



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to