Concurrency/Parallelism vs static

Rozważmy sobie taki kod:

public abstract class Xddd
{
	private static int _InstanceCounter = 0;

	public int Id { get; } = _InstanceCounter++;
}

problem jest taki, że w testach gdy odpalimy N testów na raz, to zaczyna się robić rozjazd Id ze względu na staticness

a chciałbym uniknąć robienia jakiegoś providera licznika

typu

var result = new MójKod(new Licznik { CounterStartsAt = 0 }).DoSomething();

bo ciężko widzę rzucanie tym licznikiem po całym codebase, a z drugiej strony odpalanie testów sekwencyjnie to też niezbyt dobrze skalujące się rozwiązanie

jakiś fajny pomysł jak to ugrać?

https://docs.microsoft.com/en-us/dotnet/api/system.threading.interlocked.increment?view=net-5.0

@Afish

że tak?

public class xDD
{
    private static int _InstanceCounter = 0;

    public int Id { get; } = Interlocked.Increment(ref _InstanceCounter);
}

[Fact]
public void T1()
{
	var _1 = new xDD();
	Assert.Equal(1, _1.Id);
	var _2 = new xDD();
	Assert.Equal(2, _2.Id);
}

[Fact]
public void T2()
{
	var _1 = new xDD();
	Assert.Equal(1, _1.Id);
	var _2 = new xDD();
	Assert.Equal(2, _2.Id);
}

[Fact]
public void T3()
{
	var _1 = new xDD();
	Assert.Equal(1, _1.Id);
	var _2 = new xDD();
	Assert.Equal(2, _2.Id);
}

wydaje mi się że lock i podobne tu nie zadziałają, bo problemem jest że testy sharują wartość statyczną

muszę przyznać, że dawno takiego haka nie widziałem, ale koniec końców i tak muszę całość jakoś lepiej przerobić, bo poza testami i tak może wystąpić ten problem

public class Test232
{
	public Test232()
	{
		xDD._InstanceCounter = 0;
	}

	[Fact]
	...
}

Źle zrozumiałem, co Ty tworzysz. Teraz rozumiem i łapki opadają…
Po co chcesz mieć static i jednocześnie odpalać rzeczy równolegle? To się kupy nie trzyma.

@Afish:

no wiem że się nie trzyma, dlatego tu piszę :D

Chciałem sobie pójść na łatwiznę i poprzez

private static int _InstanceCounter = 0;

generować unikatowe, przejrzyste, a dodatkowo odtwarzalne/reproducible id instancji, a bonusowe punkty za dwie linijki kodu

X Y; I guess

Jak masz dobre testy to powinieneś tam móc wstawić dowolną wartość.
No chyba że dla jednej wygenerowanej wartości wielokrotnie ją odczytujesz, to wtedy masz po prostu burdel.
To zerowanie wartości świadczy o tym że coś tam brzydko pachnie.

Challenge: przed każdym testem wstaw tam losową wartość. Test powinien się udać.

@vpiotr

Jak masz dobre testy to powinieneś tam móc wstawić dowolną wartość.

jako że dawanie dalej przykładów na iksach dupiksach chyba nie ma sensu, to idźmy do czegoś konkretniejszego

Mamy test parsera xmli, zamierzam go testować tylko i wyłącznie całościowo, bez rozbijania na jakieś unit test pojedynczych funkcyjek

[Fact]
public void Test1()
{
	var xml = @"<dupa>asd</dupa>";
	var engine = new XmlEngine();
	var tree = engine.GenerateTree(xml);

	Assert.Equals(1, tree.Root.Id);
	Assert.True("asd", tree.Root.Value);
}

GenerateTree zwraca drzewo zbudowane z różnych Node (parent class), ale chciałbym aby każdy z nich miał unikatowe, przejrzyste, a dodatkowo odtwarzalne/reproducible id instancji

Jako że pod spodem jest dużo różnych mniejszych "enginów" czy ogólnie miejsc, gdzie jest robiony new *Node(), to bardzo zależało by mi aby uniknąć jakiegoś rozwiązania, które polegałoby na przerzucaniu jakimiś obiektami typu Settings po całym codebase

Problem jest taki, że gdy puszczam testy, to w podejściu z 1 posta (które jest bardzo przyjemne bo mało kodu i działa OK) te statiki na siebie nachodzą

W dodatku tak sobie jeszcze myślę, że gdy w tym GenerateTree będę robił coś Parallel.Forem to też się natknę na ten problem jak przy testach

OK, a czy na pewno musisz testować to ID?
Chyba ostatnia rzecz jaką powinieneś testować to kolejność generacji ID?
Nie lepiej name/value/jakaś inna wartość?

Jak chcesz koniecznie testować ID to raczej zostaje Ci wykonanie sekwencyjne testów (chyba że C# ma jakiś superwynalazek, ale to może ktoś tu jeszcze napisze).

@vpiotr:

OK, a czy na pewno musisz testować to ID?
Chyba ostatnia rzecz jaką powinieneś testować to kolejność generacji ID?
Nie lepiej name/value/jakaś inna wartość?

no zależałoby mi, bo później sobie generuje wizualizacje na podstawie tego

digraph Tree
{
	{"0" [label="Root"]} -> {"1" [label="'dupa'"]}
	{"1" [label="'dupa'"]} -> {"2" [label="'dupa1'"]}
	{"1" [label="'dupa'"]} -> {"3" [label="'dupa2'"]}
}

Jak bym miał to dzisiaj napisać to użyłbym ConcurrentDictionary (z kluczem = Thread.CurrentThread.ManagedThreadId), ale zaznaczam że nie znam C# i mogą być lepsze sposoby.

https://docs.microsoft.com/pl-pl/dotnet/standard/collections/thread-safe/how-to-add-and-remove-items

Mi się tutaj kłóci Parallel.For z wymaganiem, że ma być odtwarzalne/reproducible, bo przecież nie ma gwarancji w jakiej kolejności ponumerują się węzły. Ale rozumiem, że to właśnie jest problemem tego wątku.
Wydaje mi się, że jeśli chcesz przetwarzać te dane równolegle to numery węzłom musisz nadać już po przeparsowaniu się całego ciągu wejściowego, aby to było powtarzalne i deterministyczne. A i do tego nie użyłbym pola statycznego, być może jakąś rekursją?

@vpiotr

Pomysł na pewno ciekawy, bo rozważałem podobny z taką różnicą, że każda instancja XmlEngine miałaby swój własny Guid, który byłby następnie używany w tym Dictionary, ale problem był taki że trzeba byłoby nim przerzucać po całym projekcie :P Pomysł z ThreadId wydaje się być sensowny, ale nie trybi mi coś w testach

Idąc dalej, słusznie @maszrum zwrócił uwagę, że i tak trzeba będzie odroczyć moment nadania Id, więc na razie wpadłem na taki pomysł

public class Node
{
	public Guid Internal_Id { get; } = Guid.NewGuid();

	public int? Short_Id { get; set; }
}

Internal_Id - będzie autogenerowane, unikatowe i będzie pozwalało zręczniej poruszać czy operować na drzewie

Short_Id - będzie unikatowy oraz generowany w momencie gdy już sama struktura drzewa się ustabilizuje, dzięki czemu powinien być odtwarzalny, a dodatkowo przejrzysty bo to zwykły int i posłuży do generowania wizualizacji

Dzięki.

Użyj AsyncLocal - pozwala na przypisanie staticów przywiązanych do aktualnego "flow" kodu, działa jak "ThreadStatic", ale przykładowo nie rozjedzie się po użyciu "async" / "await" z ConfigureAwait(false) po przerzuceniu flowa na inny wątek.

Reset licznika zrób w Setup testu tak jak to robisz teraz (konstruktor dla xunit), ale przy użyciu AsyncLocal masz pewność że każdy test dostanie własny licznik jeśli jest wykonywany w tym samym czasie i nie będą ze sobą kolidowały.

Ale ogólnie rozwiązanie z d... ten licznik powinien należeć do XmlEngine i kontekst być niestety przekazywany do każdego obiektu pod spodem (możesz przekazywać tylko referencję do XmlEngine albo lepiej jakiś obiekt kontekstowy)

@WeiXiao: zrób sobie metodkę, która przebudowuje drzewo XML nadając IDki od nowa i wtedy możesz je w miarę sensownie testować.