Konstruktor mit nicht-const-Argument kopieren, das durch Thread-Sicherheitsregeln vorgeschlagen wird?

9

Ich habe einen Wrapper für einen alten Code.

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

In diesem Legacy-Code ist die Funktion, die ein Objekt „dupliziert“, nicht threadsicher (wenn dasselbe erste Argument aufgerufen wird), daher wird sie constim Wrapper nicht markiert . Ich schätze folgende moderne Regeln: https://herbsutter.com/2013/01/01/video-you-dont-know-const-and-mutable/

Dies duplicatescheint eine gute Möglichkeit zu sein, einen Kopierkonstruktor zu implementieren, mit Ausnahme der Details, die es nicht sind const. Deshalb kann ich das nicht direkt machen:

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

Was ist der Ausweg aus dieser paradoxen Situation?

(Nehmen wir auch an, dass dies legacy_duplicatenicht threadsicher ist, aber ich weiß, dass das Objekt beim Beenden im ursprünglichen Zustand bleibt. Als C-Funktion ist das Verhalten nur dokumentiert, hat aber kein Konzept der Konstanz.)

Ich kann mir viele mögliche Szenarien vorstellen:

(1) Eine Möglichkeit besteht darin, dass es überhaupt keine Möglichkeit gibt, einen Kopierkonstruktor mit der üblichen Semantik zu implementieren. (Ja, ich kann das Objekt bewegen und das ist nicht das, was ich brauche.)

(2) Andererseits ist das Kopieren eines Objekts von Natur aus nicht threadsicher in dem Sinne, dass das Kopieren eines einfachen Typs die Quelle in einem halbmodifizierten Zustand finden kann, also kann ich einfach weitermachen und dies vielleicht tun,

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) oder deklarieren Sie einfach duplicateconst und lügen Sie über Thread-Sicherheit in allen Kontexten. (Schließlich kümmert sich die Legacy-Funktion nicht darum, constsodass sich der Compiler nicht einmal beschwert.)

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

(4) Schließlich kann ich der Logik folgen und einen Kopierkonstruktor erstellen, der ein Nicht-Konstanten- Argument verwendet.

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

Es stellt sich heraus, dass dies in vielen Kontexten funktioniert, da diese Objekte normalerweise nicht funktionieren const.

Die Frage ist, ist dies eine gültige oder gemeinsame Route?

Ich kann sie nicht benennen, aber ich erwarte intuitiv viele Probleme auf dem Weg zu einem nicht konstanten Kopierkonstruktor. Wahrscheinlich wird es aufgrund dieser Subtilität nicht als Werttyp qualifiziert.

(5) Obwohl dies ein Overkill zu sein scheint und hohe Laufzeitkosten verursachen könnte, könnte ich einen Mutex hinzufügen:

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

Aber dazu gezwungen zu sein, sieht nach Pessimisierung aus und macht die Klasse größer. Ich bin mir nicht sicher. Ich neige derzeit zu (4) oder (5) oder einer Kombination aus beiden.

- BEARBEITEN

Andere Option:

(6) Vergessen Sie den Unsinn der doppelten Elementfunktion und rufen Sie einfach legacy_duplicatevom Konstruktor auf und erklären Sie, dass der Kopierkonstruktor nicht threadsicher ist. (Und wenn nötig, machen Sie eine andere thread-sichere Version des Typs. A_mt)

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

BEARBEITEN 2

Dies könnte ein gutes Modell für die Funktionsweise der Legacy-Funktion sein. Beachten Sie, dass der Aufruf durch Berühren der Eingabe in Bezug auf den durch das erste Argument dargestellten Wert nicht threadsicher ist.

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
quelle
1
" In diesem Legacy-Code ist die Funktion, die ein Objekt dupliziert, nicht threadsicher (wenn Sie dasselbe erste Argument aufrufen). " Sind Sie sich da sicher? Gibt es einen nicht enthaltenen Status, Lder durch Erstellen einer neuen LInstanz geändert wird ? Wenn nicht, warum glauben Sie, dass dieser Vorgang nicht threadsicher ist?
Nicol Bolas
Ja, das ist die Situation. Es sieht so aus, als ob der interne Status des ersten Arguments während der Exektion geändert wird. Aus irgendeinem Grund (eine "Optimierung" oder ein schlechtes Design oder einfach durch Spezifikation) kann die Funktion legacy_duplicatenicht mit demselben ersten Argument aus zwei verschiedenen Threads aufgerufen werden.
alfC
@ TedLyngmo ok ich habe. Obwohl technisch in c ++ vor 11 const eine unschärfere Bedeutung hat, wenn Threads vorhanden sind.
17.
@ TedLyngmo ja, es ist ein ziemlich gutes Video. Es ist schade, dass sich das Video nur mit richtigen Mitgliedern befasst und das Konstruktionsproblem nicht berührt (wo auch die Konstanz auf dem „anderen“ Objekt liegt). In der Perspektive gibt es möglicherweise keine Möglichkeit, diesen Wrapper-Thread beim Kopieren sicher zu machen, ohne eine weitere Abstraktionsebene (und einen konkreten Mutex) hinzuzufügen.
alfC
Ja, das hat mich verwirrt und ich bin wahrscheinlich einer von denen, die nicht wissen, was constwirklich bedeutet. :-) Ich würde nicht zweimal darüber nachdenken, einen const&in meine Kopie aufzunehmen, solange ich nichts ändere other. Ich denke immer an Thread-Sicherheit als etwas, das man zusätzlich zu dem hinzufügt, auf das über mehrere Kapseln über Kapselung zugegriffen werden muss, und ich freue mich sehr auf die Antworten.
Ted Lyngmo

Antworten:

0

Ich würde nur Ihre beiden Optionen (4) und (5) einschließen, mich aber explizit für thread-unsicheres Verhalten entscheiden, wenn Sie der Meinung sind, dass dies für die Leistung erforderlich ist.

Hier ist ein vollständiges Beispiel.

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

Ausgabe:

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

Dies folgt dem Google Style Guide, in dem die constThread-Sicherheit kommuniziert wird. Code, der Ihre API aufruft, kann sich jedoch mit deaktivierenconst_cast

Michael Graczyk
quelle
Vielen Dank für die Antwort, ich denke, es ändert nichts an Ihrer Antwort und ich bin nicht sicher, aber ein besseres Modell für legacy_duplicatekönnte sein 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; }(dh nicht-const in)
alfC
Ihre Antwort ist sehr interessant, da sie mit Option (4) und einer expliziten Version von Option (2) kombiniert werden kann. Das heißt, A a2(a1)könnte versuchen, threadsicher zu sein (oder gelöscht zu werden) und A a2(const_cast<A&>(a1))würde überhaupt nicht versuchen, threadsicher zu sein.
18.
2
Ja, wenn Sie vorhaben, Asowohl in thread-sicheren als auch in thread-unsicheren Kontexten zu verwenden, sollten Sie den const_castauf den aufrufenden Code ziehen, damit klar ist, wo bekanntermaßen die Thread-Sicherheit verletzt wird. Es ist in Ordnung, zusätzliche Sicherheit hinter die API (Mutex) zu stellen, aber nicht in Ordnung, die Unsicherheit zu verbergen (const_cast).
Michael Graczyk
0

TLDR: Entweder die Umsetzung Ihrer Doppelfunktion zu beheben, oder ein Mutex einführen (oder eine geeignetere Verriegelungsvorrichtung, vielleicht ein spinlock, oder stellen Sie sicher , Ihre Mutex Spin konfiguriert ist , bevor etwas schwerer tun) für jetzt , beheben dann die Umsetzung der Vervielfältigung und entfernen Sie die Verriegelung, wenn die Verriegelung tatsächlich zu einem Problem wird.

Ich denke, ein wichtiger Punkt ist, dass Sie eine Funktion hinzufügen, die vorher nicht existierte: die Möglichkeit, ein Objekt aus mehreren Threads gleichzeitig zu duplizieren.

Unter den von Ihnen beschriebenen Bedingungen wäre dies natürlich ein Fehler gewesen - eine Rennbedingung, wenn Sie dies zuvor getan hätten, ohne irgendeine externe Synchronisation zu verwenden.

Daher wird jede Verwendung dieser neuen Funktion etwas, das Sie Ihrem Code hinzufügen und nicht als vorhandene Funktionalität erben. Sie sollten derjenige sein, der weiß, ob das Hinzufügen der zusätzlichen Sperre tatsächlich kostspielig ist - je nachdem, wie oft Sie diese neue Funktion verwenden werden.

Aufgrund der wahrgenommenen Komplexität des Objekts - aufgrund der speziellen Behandlung, die Sie ihm geben - gehe ich davon aus, dass das Duplizierungsverfahren nicht trivial ist und daher in Bezug auf die Leistung bereits recht teuer ist.

Basierend auf dem oben Gesagten haben Sie zwei Wege, denen Sie folgen können:

A) Sie wissen, dass das Kopieren dieses Objekts aus mehreren Threads nicht oft genug erfolgt, um den Aufwand für die zusätzliche Sperrung zu erhöhen - möglicherweise trivial billig, zumindest angesichts der Tatsache, dass das vorhandene Duplizierungsverfahren allein teuer genug ist, wenn Sie a verwenden Spinlock / Pre-Spinning-Mutex, und es gibt keinen Streit darüber.

B) Sie vermuten, dass das Kopieren von mehreren Threads häufig genug erfolgt, damit das zusätzliche Sperren ein Problem darstellt. Dann haben Sie wirklich nur eine Option - korrigieren Sie Ihren Duplizierungscode. Wenn Sie das Problem nicht beheben, müssen Sie es trotzdem sperren, sei es auf dieser Abstraktionsebene oder anderswo, aber Sie benötigen es, wenn Sie keine Fehler möchten - und wie wir in diesem Pfad festgestellt haben, nehmen Sie an Dieses Sperren ist zu kostspielig. Daher besteht die einzige Möglichkeit darin, den Duplizierungscode zu korrigieren.

Ich vermute, dass Sie sich wirklich in Situation A befinden und nur einen Spinlock / Spinning-Mutex hinzufügen, der im unbestrittenen Zustand nahezu keine Leistungseinbußen aufweist, funktioniert einwandfrei (denken Sie jedoch daran, ihn zu bewerten).

Theoretisch gibt es eine andere Situation:

C) Im Gegensatz zu der scheinbaren Komplexität der Duplizierungsfunktion ist sie eigentlich trivial, kann aber aus irgendeinem Grund nicht behoben werden. Es ist so trivial, dass selbst ein unbestrittener Spinlock zu einer inakzeptablen Leistungsverschlechterung bei der Duplizierung führt. Duplikate auf Parallell-Threads werden selten verwendet. Die Duplizierung auf einem einzelnen Thread wird ständig verwendet, wodurch die Leistungsverschlechterung absolut inakzeptabel wird.

In diesem Fall schlage ich Folgendes vor: Deklarieren Sie die Standardkopierkonstruktoren / -operatoren als gelöscht, um zu verhindern, dass jemand sie versehentlich verwendet. Erstellen Sie zwei explizit aufrufbare Duplizierungsmethoden, eine thread-sichere und eine thread-unsichere. Lassen Sie Ihre Benutzer sie je nach Kontext explizit aufrufen. Auch hier gibt es keine andere Möglichkeit, eine akzeptable Single-Thread-Leistung und ein sicheres Multi-Threading zu erzielen, wenn Sie sich wirklich in dieser Situation befinden und die vorhandene Duplizierungsimplementierung einfach nicht reparieren können . Aber ich halte es für sehr unwahrscheinlich, dass Sie es wirklich sind.

Fügen Sie einfach diesen Mutex / Spinlock und Benchmark hinzu.

DeducibleSteak
quelle
Können Sie mich auf Material zu Spinlock / Pre-Spinning-Mutex in C ++ verweisen? Ist es etwas komplizierter als das, was von bereitgestellt wird std::mutex? Die Duplikatfunktion ist kein Geheimnis, ich habe sie nicht erwähnt, um das Problem auf einem hohen Niveau zu halten und keine Antworten über MPI zu erhalten. Aber da Sie so tief gegangen sind, kann ich Ihnen mehr Details geben. Die Legacy-Funktion ist MPI_Comm_dupund die effektive Sicherheit ohne Thread wird hier beschrieben (ich habe es bestätigt). Github.com/pmodels/mpich/issues/3234 . Aus diesem Grund kann ich Duplikate nicht reparieren. (Wenn ich einen Mutex hinzufüge, werde ich versucht sein, alle MPI-Aufrufe threadsicher zu machen.)
alfC
Leider weiß ich nicht viel über std :: mutex, aber ich denke, es dreht sich etwas, bevor der Prozess schlafen geht. Ein bekanntes Synchronisationsgerät, mit dem Sie dies manuell steuern können, ist: docs.microsoft.com/en-us/windows/win32/api/synchapi/… Ich habe die Leistung nicht verglichen, aber es scheint, dass std :: mutex dies ist jetzt überlegen: stackoverflow.com/questions/9997473/… und implementiert mit: docs.microsoft.com/en-us/windows/win32/sync/…
DeducibleSteak
Es scheint, dass dies eine gute Beschreibung der allgemeinen zu berücksichtigenden Überlegungen ist
DeducibleSteak
Nochmals vielen Dank, ich bin in Linux, wenn das wichtig ist.
20.
Hier ist ein etwas detaillierter Leistungsvergleich (für eine andere Sprache, aber ich denke, dies ist informativ und zeigt an, was zu erwarten ist): matklad.github.io/2020/01/04/… Der TLDR ist - Spinlocks gewinnen durch einen extrem kleinen Marge, wenn es keinen Streit gibt, kann schlecht verlieren, wenn es Streit gibt.
DeducibleSteak