Copiar construtor com argumento não const sugerido por regras de segurança de thread?

9

Eu tenho um wrapper para algum pedaço de código legado.

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
};

Nesse código legado, a função que "duplica" um objeto não é segura para threads (ao chamar o mesmo primeiro argumento), portanto, não está marcada const no wrapper. Eu acho que seguindo as regras modernas: https://herbsutter.com/2013/01/01/video-you-dont-know-const-and-mutable/

Esta duplicateparece ser uma boa maneira de implementar um construtor de cópia, exceto para o detalhe que não é const. Portanto, não posso fazer isso diretamente:

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;}
};

Então, qual é a saída dessa situação paradoxal?

(Digamos também que legacy_duplicatenão é seguro para threads, mas eu sei que deixa o objeto no estado original quando ele sai. Sendo uma função C, o comportamento é apenas documentado, mas não tem conceito de constância.)

Eu posso pensar em muitos cenários possíveis:

(1) Uma possibilidade é que não há como implementar um construtor de cópias com a semântica usual. (Sim, eu posso mover o objeto e não é disso que preciso.)

(2) Por outro lado, copiar um objeto é inerentemente não seguro para threads, no sentido de que copiar um tipo simples pode encontrar a fonte em um estado semi-modificado, para que eu possa ir adiante e fazer isso talvez,

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) ou até mesmo declarar duplicateconst e mentir sobre segurança de threads em todos os contextos. (Afinal, a função legada não se importa, constentão o compilador nem se queixará.)

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

(4) Finalmente, posso seguir a lógica e criar um construtor de cópias que usa um argumento não-const .

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

Acontece que isso funciona em muitos contextos, porque esses objetos geralmente não são const .

A questão é: esta é uma rota válida ou comum?

Não posso nomeá-los, mas, intuitivamente, espero muitos problemas no caminho de ter um construtor de cópias não-const. Provavelmente não se qualificará como um tipo de valor por causa dessa sutileza.

(5) Finalmente, embora isso pareça um exagero e possa ter um alto custo de tempo de execução, eu poderia adicionar um 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;
};

Mas ser forçado a fazer isso parece pessimização e aumenta a classe. Não tenho certeza. Atualmente, estou inclinado a (4) ou (5) ou a uma combinação de ambos.

—— EDITAR

Outra opção:

(6) Esqueça toda a falta de sentido da função de membro duplicado e simplesmente chame legacy_duplicateo construtor e declare que o construtor de cópia não é seguro para threads. (E, se necessário, faça outra versão com segurança para threads do tipo A_mt)

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

EDIT 2

Este poderia ser um bom modelo para o que a função legada faz. Observe que, ao tocar na entrada, a chamada não é segura com relação ao valor representado pelo primeiro 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
fonte
11
" Neste código legado, a função que duplica um objeto não é segura para threads (ao chamar o mesmo primeiro argumento) " Você tem certeza disso? Existe algum estado não contido no Lqual é modificado criando uma nova Linstância? Caso contrário, por que você acredita que esta operação não é segura para threads?
Nicol Bolas
Sim, essa é a situação. Parece que o estado interno do primeiro argumento foi modificado durante a exceção. Por alguma razão (alguma "otimização" ou design incorreto ou simplesmente por especificação), a função legacy_duplicatenão pode ser chamada com o mesmo primeiro argumento de dois threads diferentes.
alfC
@TedLyngmo ok eu fiz. Embora tecnicamente em c ++ pre 11 const tenha um significado mais nebuloso na presença de threads.
alfC 17/02
@TedLyngmo sim, é um vídeo muito bom. é uma pena que o vídeo lide apenas com membros apropriados e não toque no problema de construção (também a constância está no "outro" objeto). Em perspectiva, pode não haver uma maneira intrínseca de tornar esse encapsulamento seguro ao copiar sem adicionar outra camada de abstração (e um mutex concreto).
alfC
Sim, isso me deixou confuso e provavelmente sou uma daquelas pessoas que não sabem o que constrealmente significa. :-) Eu não pensaria duas vezes em tirar um const&cópia no meu copiador, desde que não o modifique other. Eu sempre penso na segurança de threads como algo que se acrescenta sobre o que precisa ser acessado de vários threads, via encapsulamento, e estou realmente ansioso pelas respostas.
Ted Lyngmo

Respostas:

0

Eu incluiria apenas as suas opções (4) e (5), mas explicitamente optarei pelo comportamento não seguro do thread quando você achar necessário para o desempenho.

Aqui está um exemplo 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);
}

Resultado:

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

Isso segue o guia de estilo do Google no qual se constcomunica a segurança do encadeamento, mas o código que chama sua API pode desativar usandoconst_cast

Michael Graczyk
fonte
Obrigado pela resposta, acho que isso não muda sua resposta e não tenho certeza, mas legacy_duplicatepoderia ser um modelo melhor 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; }(isto é, não-const in)
alfC
Sua resposta é muito interessante porque pode ser combinada com a opção (4) e uma versão explícita da opção (2). Ou seja, A a2(a1)poderia tentar ser seguro para threads (ou ser excluído) e A a2(const_cast<A&>(a1))não tentaria ser seguro para threads.
alfC 18/02
2
Sim, se você planeja usar Anos contextos thread-safe e inseguro, você deve puxar o const_castcódigo de chamada para que fique claro onde é sabido que a segurança do thread é violada. Não há problema em colocar segurança extra atrás da API (mutex), mas não há problema em esconder a segurança (const_cast).
Michael Graczyk 18/02
0

TLDR: corrija a implementação da sua função de duplicação ou introduza um mutex (ou algum dispositivo de bloqueio mais apropriado, talvez um spinlock) ou verifique se o mutex está configurado para girar antes de fazer algo mais pesado por enquanto , em seguida, corrija a implementação da duplicação e remova o bloqueio quando o bloqueio realmente se tornar um problema.

Acho que um ponto importante a ser observado é que você está adicionando um recurso que não existia antes: a capacidade de duplicar um objeto de vários threads ao mesmo tempo.

Obviamente, nas condições que você descreveu, isso seria um bug - uma condição de corrida, se você estivesse fazendo isso antes, sem usar algum tipo de sincronização externa.

Portanto, qualquer uso desse novo recurso será algo que você adicionará ao seu código, não herdará como funcionalidade existente. Você deve saber quem adicionar o bloqueio extra será realmente caro - dependendo da frequência com que você usará esse novo recurso.

Além disso, com base na complexidade percebida do objeto - pelo tratamento especial que você está dando a ele, vou assumir que o procedimento de duplicação não é trivial, portanto, já é bastante caro em termos de desempenho.

Com base no exposto, você tem dois caminhos a seguir:

A) Você sabe que copiar este objeto de vários encadeamentos não acontecerá com frequência suficiente para que a sobrecarga do bloqueio adicional seja dispendiosa - provavelmente trivialmente barata, pelo menos uma vez que o procedimento de duplicação existente é dispendioso por si só, se você usar um mutex spinlock / pré-spinning, e não há contenção nele.

B) Você suspeita que a cópia de vários encadeamentos acontecerá com frequência suficiente para que o bloqueio extra seja um problema. Então você realmente tem apenas uma opção - corrija seu código de duplicação. Se você não corrigi-lo, precisará bloquear de qualquer maneira, seja nesta camada de abstração ou em outro lugar, mas será necessário se não desejar erros - e, como estabelecemos, nesse caminho, você assume esse bloqueio será muito caro, portanto, a única opção é corrigir o código de duplicação.

Eu suspeito que você esteja realmente na situação A, e apenas adicionar um mutex spinlock / spinning que tenha quase nenhuma penalidade de desempenho quando não contestado funcionará muito bem (lembre-se de compará-lo).

Existe, em teoria, outra situação:

C) Em contraste com a aparente complexidade da função de duplicação, ela é realmente trivial, mas não pode ser corrigida por algum motivo; é tão trivial que mesmo um spinlock não contestado introduz uma degradação inaceitável do desempenho na duplicação; a duplicação em threads paralelos é usada raramente; a duplicação em um único encadeamento é usada o tempo todo, tornando a degradação do desempenho absolutamente inaceitável.

Nesse caso, sugiro o seguinte: declare os construtores / operadores de cópia padrão excluídos, para impedir que alguém os use acidentalmente. Crie dois métodos de duplicação explicitamente solicitáveis, um segmento seguro e outro não seguro; faça seus usuários chamá-los explicitamente, dependendo do contexto. Novamente, não há outra maneira de obter desempenho aceitável de encadeamento único e multiencadeamento seguro, se você realmente estiver nessa situação e simplesmente não conseguir consertar a implementação de duplicação existente. Mas eu sinto que é altamente improvável que você realmente seja.

Basta adicionar esse mutex / spinlock e referência.

DeducibleSteak
fonte
Você pode me indicar material sobre mutex spinlock / pré-spinning em C ++? É algo mais complicado que o que é fornecido std::mutex? A função duplicada não é segredo, não mencionei para manter o problema em um nível alto e não receber respostas sobre o MPI. Mas desde que você foi tão fundo, eu posso lhe dar mais detalhes. A função legada é MPI_Comm_dupe a segurança efetiva sem thread é descrita aqui (confirmei) github.com/pmodels/mpich/issues/3234 . É por isso que não consigo corrigir duplicados. (Além disso, se eu adicionar um mutex, ficarei tentado a fazer com que todas as chamadas MPI sejam seguras contra threads.)
alfC
Infelizmente, eu não conheço muito std :: mutex, mas acho que ele gira um pouco antes de deixar o processo dormir. Um dispositivo de sincronização conhecido em que você pode controlar isso manualmente é: docs.microsoft.com/en-us/windows/win32/api/synchapi/… Não comparei o desempenho, mas parece que std :: mutex é agora superior: stackoverflow.com/questions/9997473/… e implementado usando: docs.microsoft.com/en-us/windows/win32/sync/…
DeducibleSteak
Parece que esta é uma boa descrição das considerações gerais a serem
levadas
Mais uma vez obrigado, estou no Linux, se isso importa.
alfC 20/02
Aqui está uma comparação de desempenho um pouco detalhada (para um idioma diferente, mas acho que isso é informativo e indicativo do que esperar): matklad.github.io/2020/01/04/… O TLDR é - os spinlocks vencem por um tamanho extremamente pequeno margem quando não há contenção, pode perder muito quando há contenção.
DeducibleSteak