Skopiować konstruktor z argumentem non-const sugerowanym przez reguły bezpieczeństwa wątków?


9

Mam opakowanie jakiegoś starszego kodu.

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

W tym starszym kodzie funkcja „duplikująca” obiekt nie jest bezpieczna dla wątków (podczas wywoływania tego samego pierwszego argumentu), dlatego nie jest oznaczona constw opakowaniu. Chyba następujące współczesne zasady: https://herbsutter.com/2013/01/01/video-you-dont-know-const-and-mutable/

To duplicatewygląda na dobry sposób na implementację konstruktora kopii, z wyjątkiem szczegółów, które nie są const. Dlatego nie mogę tego zrobić bezpośrednio:

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

Jakie jest zatem wyjście z tej paradoksalnej sytuacji?

(Powiedzmy również, że legacy_duplicatenie jest to bezpieczne dla wątków, ale wiem, że opuszcza obiekt w oryginalnym stanie po wyjściu. Jako funkcja C zachowanie jest tylko udokumentowane, ale nie ma pojęcia stałości).

Mogę wymyślić wiele możliwych scenariuszy:

(1) Jedną z możliwości jest to, że nie ma możliwości zaimplementowania konstruktora kopii o zwykłej semantyce. (Tak, mogę przesunąć obiekt i to nie jest to, czego potrzebuję.)

(2) Z drugiej strony, kopiowanie obiektu z natury nie jest bezpieczne w tym sensie, że kopiowanie prostego typu może znaleźć źródło w stanie częściowo zmodyfikowanym, więc mogę po prostu iść do przodu i zrobić to, być może,

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) lub nawet po prostu deklarować duplicateconst i kłamać na temat bezpieczeństwa wątków we wszystkich kontekstach. (Po tym wszystkim, na czym polega starsza funkcja, constkompilator nawet nie narzeka.)

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

(4) Na koniec mogę postępować zgodnie z logiką i stworzyć konstruktor kopii, który przyjmuje argument nie-stały .

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

Okazuje się, że działa to w wielu kontekstach, ponieważ te obiekty zwykle nie są const.

Pytanie brzmi, czy jest to ważna czy wspólna trasa?

Nie potrafię ich nazwać, ale intuicyjnie oczekuję wielu problemów na drodze do posiadania konstruktora kopii non-const. Prawdopodobnie nie będzie się kwalifikować jako typ wartości z powodu tej subtelności.

(5) Wreszcie, chociaż wydaje się to przesadą i może mieć wysokie koszty w czasie wykonywania, mogę dodać muteks:

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

Ale zmuszenie do zrobienia tego wygląda na pesymalizację i powiększa klasę. Nie jestem pewien. Obecnie skłaniam się w kierunku (4) lub (5) lub kombinacji obu.


EDYCJA 1:

Inna opcja:

(6) Zapomnij o braku zrozumienia funkcji duplikatu elementu i po prostu wywołaj legacy_duplicatez konstruktora i zadeklaruj, że konstruktor kopiowania nie jest bezpieczny dla wątków. (I jeśli to konieczne, wykonaj kolejną bezpieczną dla wątku wersję tego typu A_mt).

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

EDYCJA 2:

Może to być dobry model do tego, co robi starsza funkcja. Zauważ, że dotknięcie wejścia powoduje, że wywołanie nie jest bezpieczne dla wątku w odniesieniu do wartości reprezentowanej przez pierwszy argument.

void legacy_duplicate(L* in, L** out){
   *out = new L{};
   char tmp = in[0];
   in[0] = tmp; 
   std::memcpy(*out, in, sizeof *in); return; 
}

EDYCJA 3: Niedawno dowiedziałem się, że std::auto_ptrmiał podobny problem z konstruktorem „kopiowania” innym niż const. Efekt był taki, że auto_ptrnie można go użyć w pojemniku. https://www.quantstart.com/articles/STL-Containers-and-Auto_ptrs-Why-They-Dont-Mix/


1
W tym starszym kodzie funkcja duplikująca obiekt nie jest bezpieczna dla wątków (podczas wywoływania tego samego pierwszego argumentu) ” Czy jesteś tego pewien? Czy istnieje stan, który nie jest zawarty, w Lktórym jest modyfikowany przez utworzenie nowej Linstancji? Jeśli nie, dlaczego uważasz, że ta operacja nie jest bezpieczna dla wątków?
Nicol Bolas,

Tak, taka jest sytuacja. Wygląda na to, że stan wewnętrzny pierwszego argumentu został zmodyfikowany podczas exection. Z jakiegoś powodu (jakaś „optymalizacja”, zły projekt lub po prostu ze specyfikacji) funkcja legacy_duplicatenie może zostać wywołana z tym samym pierwszym argumentem z dwóch różnych wątków.
alfC

@TedLyngmo ok zrobiłem. Chociaż technicznie w c ++ pre 11 const ma bardziej rozmyte znaczenie w obecności wątków.
alfC

@TedLyngmo tak, to całkiem niezły film. szkoda, że ​​wideo zajmuje się tylko właściwymi elementami i nie dotyka problemu budowy (gdyby również stałość była na „innym” obiekcie). W perspektywie może nie istnieć żaden wewnętrzny sposób zabezpieczenia tego wątku opakowującego po skopiowaniu bez dodania kolejnej warstwy abstrakcji (i konkretnego muteksu).
alfC

Tak, cóż, to mnie zdezorientowało i prawdopodobnie jestem jedną z tych osób, które nie wiedzą, co consttak naprawdę znaczy. :-) Nie zastanawiałbym się dwa razy nad pobraniem const&mojego narzędzia kopiowania, dopóki nie zmodyfikuję other. Zawsze myślę o bezpieczeństwie wątków jako o czymś, co dodaje się do wszystkiego, co wymaga dostępu z wielu wątków, poprzez enkapsulację, i naprawdę nie mogę się doczekać odpowiedzi.
Ted Lyngmo

Odpowiedzi:


0

Chciałbym tylko uwzględnić obie opcje (4) i (5), ale jednoznacznie wyrazić zgodę na zachowanie niebezpieczne dla wątków, jeśli uważasz, że jest to konieczne dla wydajności.

Oto kompletny przykład.

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

Wynik:

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

Jest to zgodne z przewodnikiem po stylu Google, w którym constkomunikuje się bezpieczeństwo wątków, ale kod wywołujący interfejs API może zrezygnowaćconst_cast


Dziękuję za odpowiedź, myślę, że to nie zmienia twojego odpowiedzi i nie jestem pewien, ale lepszym modelem legacy_duplicatemoże być 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; }(tj. Non-const in)
alfC

Twoja odpowiedź jest bardzo interesująca, ponieważ można ją połączyć z opcją (4) i jawną wersją opcji (2). Oznacza to, że A a2(a1)może próbować być bezpieczny dla wątków (lub zostać usunięty) i A a2(const_cast<A&>(a1))wcale nie próbowałby być bezpieczny dla wątków.
alfC

2
Tak, jeśli planujesz używać Azarówno w kontekście bezpiecznym dla wątków, jak i dla wątku, powinieneś pobrać const_castkod wywołujący, aby było jasne, gdzie wiadomo, że naruszono bezpieczeństwo wątków. Można wcisnąć dodatkowe bezpieczeństwo za API (mutex), ale nie jest w porządku ukryć brak bezpieczeństwa (const_cast).
Michael Graczyk

0

TLDR: Albo naprawić realizację swojej funkcji powielanie lub wprowadzić mutex (lub trochę bardziej odpowiednie urządzenie blokujące, być może Spinlock, lub upewnić się, że mutex jest skonfigurowany do korkociągu zanim cokolwiek cięższy) do teraz , a następnie naprawić realizację powielania i usuń blokadę, gdy blokowanie faktycznie stanie się problemem.

Myślę, że kluczową kwestią, na którą należy zwrócić uwagę, jest to, że dodajesz funkcję, która wcześniej nie istniała: możliwość duplikowania obiektu z wielu wątków jednocześnie.

Oczywiście w warunkach, które opisałeś, byłby to błąd - warunek wyścigu, gdybyś to robił wcześniej, bez korzystania z jakiejś zewnętrznej synchronizacji.

Dlatego użycie tej nowej funkcji będzie dodawane do kodu, a nie dziedziczone jako istniejąca funkcjonalność. Powinieneś być tym, który wie, czy dodanie dodatkowego blokowania będzie faktycznie kosztowne - w zależności od tego, jak często będziesz używać tej nowej funkcji.

Ponadto, w oparciu o postrzeganą złożoność obiektu - przez specjalne traktowanie, które mu dajesz, założę, że procedura powielania nie jest trywialna, a zatem już dość droga pod względem wydajności.

W oparciu o powyższe masz dwie ścieżki, którymi możesz podążać:

A) Wiesz, że kopiowanie tego obiektu z wielu wątków nie będzie się odbywać wystarczająco często, aby narzut dodatkowego blokowania był kosztowny - być może banalnie tani, przynajmniej biorąc pod uwagę, że istniejąca procedura kopiowania jest sama w sobie dość kosztowna, jeśli używasz muteks spinlock / pre-spinning i nie ma z tym żadnej niezgody.

B) Podejrzewasz, że kopiowanie z wielu wątków będzie się odbywać wystarczająco często, aby dodatkowe blokowanie stanowiło problem. Zatem naprawdę masz tylko jedną opcję - napraw kod duplikacji. Jeśli tego nie naprawisz, i tak będziesz musiał zablokować, czy to na tej warstwie abstrakcji, czy gdzie indziej, ale będziesz go potrzebował, jeśli nie chcesz błędów - i jak ustaliliśmy, na tej ścieżce zakładasz blokowanie będzie zbyt kosztowne, dlatego jedyną opcją jest naprawienie kodu duplikacji.

Podejrzewam, że naprawdę znajdujesz się w sytuacji A i po prostu dodanie blokady / wirującego muteksu, który nie ma żadnej obniżki wydajności, gdy jest niekwestionowany, będzie działać dobrze (pamiętaj jednak, aby go przetestować).

Teoretycznie istnieje inna sytuacja:

C) W przeciwieństwie do pozornej złożoności funkcji duplikacji, jest ona w rzeczywistości trywialna, ale z jakiegoś powodu nie można jej naprawić; jest tak trywialny, że nawet niekwestionowana blokada wprowadza niedopuszczalne pogorszenie wydajności duplikacji; powielanie wątków równoległych jest rzadko stosowane; duplikacja na jednym wątku jest używana przez cały czas, przez co pogorszenie wydajności jest absolutnie nie do przyjęcia.

W takim przypadku sugeruję, aby: zadeklarować usunięcie domyślnych konstruktorów / operatorów kopiowania, aby zapobiec przypadkowemu ich użyciu. Utwórz dwie jawnie wywoływalne metody powielania: bezpieczną dla wątku i niebezpieczną dla wątku; spraw, aby użytkownicy dzwonili do nich jawnie, w zależności od kontekstu. Ponownie, nie ma innego sposobu na osiągnięcie akceptowalnej wydajności pojedynczego wątku i bezpiecznego wielowątkowości, jeśli naprawdę jesteś w takiej sytuacji i po prostu nie możesz naprawić istniejącej implementacji duplikacji. Ale wydaje mi się, że jest mało prawdopodobne, że tak naprawdę jesteś.

Wystarczy dodać ten mutex / spinlock i test porównawczy.


Czy możesz wskazać mi materiał na temat mutloka spinlock / pre-spinning w C ++? Czy jest to coś bardziej skomplikowanego niż to, co zapewnia std::mutex? Funkcja duplikatu nie jest tajemnicą, nie wspominałem o niej, aby utrzymać problem na wysokim poziomie i nie otrzymywać odpowiedzi na temat MPI. Ale odkąd poszedłeś tak głęboko, mogę podać ci więcej szczegółów. Jest to funkcja starszego typu, MPI_Comm_dupa efektywne bezpieczeństwo nie-wątkowe jest opisane tutaj (potwierdziłem to) github.com/pmodels/mpich/issues/3234 . Dlatego nie mogę naprawić duplikatu. (Ponadto, jeśli dodam muteks, pokusę się, aby wszystkie połączenia MPI były bezpieczne dla wątków.)
alfC

Niestety nie znam zbyt wiele std :: mutex, ale myślę, że to trochę kręci przed uśpieniem procesu. Dobrze znanym urządzeniem do synchronizacji, w którym można to kontrolować ręcznie, jest: docs.microsoft.com/en-us/windows/win32/api/synchapi/ ... Nie porównałem wydajności, ale wygląda na to, że std :: mutex to teraz lepszy: stackoverflow.com/questions/9997473/… i zaimplementowany przy użyciu: docs.microsoft.com/en-us/windows/win32/sync/…
DeducibleSteak

Wydaje się, że to dobry opis ogólnych uwag, które należy wziąć pod uwagę: stackoverflow.com/questions/5869825/…
DeducibleSteak

Jeszcze raz dziękuję, jestem w systemie Linux, jeśli to ma znaczenie.
alfC

Oto nieco szczegółowe porównanie wydajności (dla innego języka, ale myślę, że jest to pouczające i wskazuje na to, czego się spodziewać): matklad.github.io/2020/01/04/ ... TLDR jest - spinlocki wygrywają o bardzo małe margines, gdy nie ma rywalizacji, może źle stracić, gdy jest rywalizacja.
DeducibleSteak
Korzystając z naszej strony potwierdzasz, że przeczytałeś(-aś) i rozumiesz nasze zasady używania plików cookie i zasady ochrony prywatności.
Licensed under cc by-sa 3.0 with attribution required.