Copiar constructor con argumento no constante sugerido por las reglas de seguridad de subprocesos?

9

Tengo un contenedor para alguna pieza de código heredado.

class A{
   L* impl_; // the legacy object has to be in the heap, could be also unique_ptr
   A(A const&) = delete;
   L* duplicate(){L* ret; legacy_duplicate(impl_, &L); return ret;}
   ... // proper resource management here
};

En este código heredado, la función que "duplica" un objeto no es segura para subprocesos (al invocar el mismo primer argumento), por lo tanto, no está marcada consten el contenedor. Supongo que seguir las reglas modernas: https://herbsutter.com/2013/01/01/video-you-dont-know-const-and-mutable/

Esto duplicateparece una buena manera de implementar un constructor de copia, excepto por el detalle que no es const. Por lo tanto, no puedo hacer esto directamente:

class A{
   L* impl_; // the legacy object has to be in the heap
   A(A const& other) : L{other.duplicate()}{} // error calling a non-const function
   L* duplicate(){L* ret; legacy_duplicate(impl_, &ret); return ret;}
};

Entonces, ¿cuál es la salida de esta situación paradójica?

(Digamos también que legacy_duplicateno es seguro para subprocesos, pero sé que deja el objeto en el estado original cuando sale. Al ser una función C, el comportamiento solo está documentado pero no tiene un concepto de const.)

Puedo pensar en muchos escenarios posibles:

(1) Una posibilidad es que no hay forma de implementar un constructor de copia con la semántica habitual. (Sí, puedo mover el objeto y eso no es lo que necesito).

(2) Por otro lado, copiar un objeto es inherentemente no seguro para subprocesos en el sentido de que copiar un tipo simple puede encontrar la fuente en un estado medio modificado, por lo que puedo seguir adelante y hacer esto quizás,

class A{
   L* impl_;
   A(A const& other) : L{const_cast<A&>(other).duplicate()}{} // error calling a non-const function
   L* duplicate(){L* ret; legacy_duplicate(impl_, &ret); return ret;}
};

(3) o incluso simplemente declarar duplicateconst y mentir sobre la seguridad del hilo en todos los contextos. (Después de todo, la función heredada no se preocupa por constlo que el compilador ni siquiera se quejará).

class A{
   L* impl_;
   A(A const& other) : L{other.duplicate()}{}
   L* duplicate() const{L* ret; legacy_duplicate(impl_, &ret); return ret;}
};

(4) Finalmente, puedo seguir la lógica y hacer un constructor de copias que tome un argumento no constante .

class A{
   L* impl_;
   A(A const&) = delete;
   A(A& other) : L{other.duplicate()}{}
   L* duplicate(){L* ret; legacy_duplicate(impl_, &ret); return ret;}
};

Resulta que esto funciona en muchos contextos, porque estos objetos no suelen serlo const.

La pregunta es, ¿es esta una ruta válida o común?

No puedo nombrarlos, pero intuitivamente espero muchos problemas en el futuro de tener un constructor de copia no constante. Probablemente no calificará como un tipo de valor debido a esta sutileza.

(5) Finalmente, aunque esto parece ser una exageración y podría tener un alto costo de tiempo de ejecución, podría agregar un mutex:

class A{
   L* impl_;
   A(A const& other) : L{other.duplicate_locked()}{}
   L* duplicate(){
      L* ret; legacy_duplicate(impl_, &ret); return ret;
   }
   L* duplicate_locked() const{
      std::lock_guard<std::mutex> lk(mut);
      L* ret; legacy_duplicate(impl_, &ret); return ret;
   }
   mutable std::mutex mut;
};

Pero verse obligado a hacer esto parece pesimismo y hace que la clase sea más grande. No estoy seguro. Actualmente me estoy inclinando hacia (4) o (5) o una combinación de ambos.

—— EDITAR

Otra opción:

(6) Olvídese de todo el sentido de la función miembro duplicada y simplemente llame legacy_duplicatedesde el constructor y declare que el constructor de copia no es seguro para subprocesos. (Y si es necesario, cree otra versión del tipo segura para subprocesos A_mt)

class A{
   L* impl_;
   A(A const& other){legacy_duplicate(other.impl_, &impl_);}
};

EDITAR 2

Este podría ser un buen modelo para lo que hace la función heredada. Tenga en cuenta que al tocar la entrada, la llamada no es segura para subprocesos con respecto al valor representado por el primer argumento.

void legacy_duplicate(L* in, L** out){
   *out = new L{};
   char tmp = in[0];
   in[0] = tmp; 
   std::memcpy(*out, in, sizeof *in); return; 
}
alfC
fuente
1
" En este código heredado, la función que duplica un objeto no es segura para subprocesos (al invocar el mismo primer argumento) " ¿Está seguro de eso? ¿Hay algún estado no contenido dentro del Lcual se modifica creando una nueva Linstancia? Si no es así, ¿por qué cree que esta operación no es segura para subprocesos?
Nicol Bolas
Sí, esa es la situación. Parece que el estado interno del primer argumento se modifica durante la exención. Por alguna razón (alguna "optimización" o mal diseño o simplemente por especificación) la función legacy_duplicateno se puede invocar con el mismo primer argumento de dos hilos diferentes.
AlfC
@TedLyngmo ok, lo hice. Aunque técnicamente en c ++ pre 11 const tiene un significado más difuso en presencia de hilos.
AlfC
@TedLyngmo sí, es un video bastante bueno. Es una pena que el video solo trate con los miembros apropiados y no toque el problema de la construcción (también la constidad está en el "otro" objeto). En perspectiva, puede que no haya una forma intrínseca de hacer que este hilo de envoltura sea seguro al copiar sin agregar otra capa de abstracción (y un mutex concreto).
AlfC
Sí, bueno, eso me confundió y probablemente soy una de esas personas que no sabe lo que constrealmente significa. :-) No lo pensaría dos veces antes de tomar un const&copiador en mi copiador, siempre y cuando no lo modifique other. Siempre pienso en la seguridad de los subprocesos como algo que se agrega además de cualquier cosa a la que se deba acceder desde múltiples subprocesos, mediante encapsulación, y realmente estoy esperando las respuestas.
Ted Lyngmo

Respuestas:

0

Solo incluiría sus opciones (4) y (5), pero optaría explícitamente por el comportamiento inseguro de subprocesos cuando crea que es necesario para el rendimiento.

Aquí hay un ejemplo completo.

#include <cstdlib>
#include <thread>

struct L {
  int val;
};

void legacy_duplicate(const L* in, L** out) {
  *out = new L{};
  std::memcpy(*out, in, sizeof *in);
  return;
}

class A {
 public:
  A(L* l) : impl_{l} {}
  A(A const& other) : impl_{other.duplicate_locked()} {}

  A copy_unsafe_for_multithreading() { return {duplicate()}; }

  L* impl_;

  L* duplicate() {
    printf("in duplicate\n");
    L* ret;
    legacy_duplicate(impl_, &ret);
    return ret;
  }
  L* duplicate_locked() const {
    std::lock_guard<std::mutex> lk(mut);
    printf("in duplicate_locked\n");
    L* ret;
    legacy_duplicate(impl_, &ret);
    return ret;
  }
  mutable std::mutex mut;
};

int main() {
  A a(new L{1});
  const A b(new L{2});

  A c = a;
  A d = b;

  A e = a.copy_unsafe_for_multithreading();
  A f = const_cast<A&>(b).copy_unsafe_for_multithreading();

  printf("\npointers:\na=%p\nb=%p\nc=%p\nc=%p\nd=%p\nf=%p\n\n", a.impl_,
     b.impl_, c.impl_, d.impl_, e.impl_, f.impl_);

  printf("vals:\na=%d\nb=%d\nc=%d\nc=%d\nd=%d\nf=%d\n", a.impl_->val,
     b.impl_->val, c.impl_->val, d.impl_->val, e.impl_->val, f.impl_->val);
}

Salida:

in duplicate_locked
in duplicate_locked
in duplicate
in duplicate

pointers:
a=0x7f85e8c01840
b=0x7f85e8c01850
c=0x7f85e8c01860
c=0x7f85e8c01870
d=0x7f85e8c01880
f=0x7f85e8c01890

vals:
a=1
b=2
c=1
c=2
d=1
f=2

Esto sigue la guía de estilo de Google en la que se constcomunica la seguridad del subproceso, pero el código que llama a su API puede optar por no usarconst_cast

Michael Graczyk
fuente
Gracias por la respuesta, creo que no cambia su respuesta y no estoy seguro, pero legacy_duplicatepodría ser un mejor modelo void legacy_duplicate(L* in, L** out) { *out = new L{}; char tmp = in[0]; /*some weird call here*/; in[0] = tmp; std::memcpy(*out, in, sizeof *in); return; }(es decir, no constante in)
AlfC
Su respuesta es muy interesante porque se puede combinar con la opción (4) y una versión explícita de la opción (2). Es decir, A a2(a1)podría intentar ser seguro para subprocesos (o eliminarse) y A a2(const_cast<A&>(a1))no trataría de ser seguro para subprocesos en absoluto.
AlfC
2
Sí, si planea usar Aen contextos seguros y no seguros para subprocesos, debe extraer el const_castcódigo de llamada para que quede claro dónde se sabe que se viola la seguridad de subprocesos. Está bien aplicar seguridad adicional detrás de la API (mutex) pero no está bien ocultar la inseguridad (const_cast).
Michael Graczyk
0

TLDR: arregle la implementación de su función de duplicación, o introduzca un mutex (o algún dispositivo de bloqueo más apropiado, tal vez un spinlock, o asegúrese de que su mutex esté configurado para girar antes de hacer algo más pesado) por ahora , luego arregle la implementación de la duplicación y retire el bloqueo cuando el bloqueo se convierta en un problema.

Creo que un punto clave a tener en cuenta es que está agregando una característica que no existía antes: la capacidad de duplicar un objeto de múltiples hilos al mismo tiempo.

Obviamente, bajo las condiciones que ha descrito, eso habría sido un error, una condición de carrera, si hubiera estado haciendo eso antes, sin usar algún tipo de sincronización externa.

Por lo tanto, cualquier uso de esta nueva característica será algo que agregue a su código, no heredará como funcionalidad existente. Usted debe ser quien sepa si agregar el bloqueo adicional será realmente costoso, dependiendo de la frecuencia con la que vaya a utilizar esta nueva función.

Además, según la complejidad percibida del objeto, por el tratamiento especial que le está dando, voy a suponer que el procedimiento de duplicación no es trivial, por lo tanto, ya es bastante costoso en términos de rendimiento.

Según lo anterior, tiene dos caminos que puede seguir:

R) Usted sabe que copiar este objeto desde múltiples subprocesos no sucederá con la frecuencia suficiente para que la sobrecarga del bloqueo adicional sea costosa, tal vez trivialmente barata, al menos dado que el procedimiento de duplicación existente es lo suficientemente costoso por sí solo, si utiliza un spinlock / mutex pre-spinning, y no hay contención sobre él.

B) Sospecha que la copia de varios subprocesos sucederá con la frecuencia suficiente para que el bloqueo adicional sea un problema. Entonces realmente solo tiene una opción: arreglar su código de duplicación. Si no lo arregla, necesitará bloquear de todos modos, ya sea en esta capa de abstracción o en otro lugar, pero lo necesitará si no desea errores, y como hemos establecido, en este camino, asume ese bloqueo será demasiado costoso, por lo tanto, la única opción es arreglar el código de duplicación.

Sospecho que realmente se encuentra en la situación A, y simplemente agregar un spinlock / mutex giratorio que casi no tiene penalización de rendimiento cuando no se discute, funcionará bien (recuerde marcarlo como punto de referencia, sin embargo).

Hay, en teoría, otra situación:

C) En contraste con la aparente complejidad de la función de duplicación, en realidad es trivial, pero no se puede arreglar por alguna razón; es tan trivial que incluso un spinlock sin oposición introduce una degradación inaceptable del rendimiento a la duplicación; la duplicación en hilos paralelos se usa raramente; La duplicación en un solo hilo se usa todo el tiempo, lo que hace que la degradación del rendimiento sea absolutamente inaceptable.

En este caso, sugiero lo siguiente: declarar los constructores / operadores de copia predeterminados eliminados, para evitar que alguien los use accidentalmente. Cree dos métodos de duplicación explícitamente invocables, uno seguro para subprocesos y uno no seguro para subprocesos; haga que sus usuarios los llamen explícitamente, según el contexto. Nuevamente, no hay otra manera de lograr un rendimiento aceptable de subprocesos simples y subprocesos múltiples seguros, si realmente se encuentra en esta situación y simplemente no puede solucionar la implementación de duplicación existente. Pero siento que es muy poco probable que realmente lo seas.

Simplemente agregue ese mutex / spinlock y punto de referencia.

DeducibleSteak
fuente
¿Me puede indicar material sobre spinlock / mutex pre-spinning en C ++? ¿Es algo más complicado que lo que proporciona std::mutex? La función duplicada no es un secreto, no lo mencioné para mantener el problema en un nivel alto y no recibir respuestas sobre MPI. Pero desde que fuiste tan profundo, puedo darte más detalles. La función heredada es MPI_Comm_dupy la seguridad efectiva sin subprocesos se describe aquí (lo confirmé) github.com/pmodels/mpich/issues/3234 . Es por eso que no puedo arreglar el duplicado. (Además, si agrego un mutex, estaré tentado de hacer que todas las llamadas MPI sean seguras para subprocesos).
AlfC
Lamentablemente, no sé mucho std :: mutex, pero supongo que gira un poco antes de dejar que el proceso duerma. Un dispositivo de sincronización bien conocido donde puedes controlar esto manualmente es: docs.microsoft.com/en-us/windows/win32/api/synchapi/… No he comparado el rendimiento, pero parece que std :: mutex es ahora superior: stackoverflow.com/questions/9997473/… e implementado usando: docs.microsoft.com/en-us/windows/win32/sync/…
DeducibleSteak
Parece que esta es una buena descripción de las consideraciones generales a tener en cuenta: stackoverflow.com/questions/5869825/…
DeducibleSteak
Gracias de nuevo, estoy en Linux si eso importa.
AlfC
Aquí hay una comparación de rendimiento algo detallada (para un idioma diferente, pero supongo que esto es informativo e indicativo de qué esperar): matklad.github.io/2020/01/04/… El TLDR es: los spinlocks ganan por un extremadamente pequeño margen cuando no hay contención, puede perder mucho cuando hay contención.
DeducibleSteak