Am I doing non-UI threading right?

Zenith

I'm trying to ping a list of 100 servers without waiting for each one to return as about half of them don't respond. So basically I have this code:

DataTable List = new DataTable();
List.Columns.Add("Thread", typeof(Thread));
List.Columns.Add("Response", typeof(Boolean));
foreach (String Target in Targets)
{
  Thread WTF = new Thread (Object Index)
  {
    try
    {
       PingAttempt(Target, 8);  //8 second timeout
       lock(List.SyncRoot)
       {
         List.Rows[((Integer)(Index))]["Response"] = true;
       }
    }
    catch
    {
       lock(List.SyncRoot)
       {
         List.Rows[((Integer)(Index))]["Response"] = false;
       }
    }
  }
  List.Rows.Add(WTF);
}
foreach (DataRow Target in List.Rows)
{
  ((Thread)(Target["Thread"])).Start(Target.Index);
}
foreach (DataRow Target in List.Rows)
{
  ((Thread)(Target["Thread"])).Join();
}

It's still taking forever, even though the half I can reach returned within about a minute before adding the half I can't reach. Am I spinning the threads off wrong, are pings always blocking, or is there something else I'm just missing here?

Gribnit

@Zenith too many threads, and too many locks. Use a thread pool, and a non-blocking collection of results.

Then you'll want a count-down latch for enough successes to give up, unless you have to be exhaustive.

Applied Mediocrity

@Zenith I don't like your code (), but I see no obvious cause of mischief. 100 threads firing off one packet and sleepwaiting is peanuts. You don't even need locking (the table is not being modified ~~and boolean writes are atomic~~ no, they aren't - DataTable is boxing), but it won't be the problem either.

C# Ping class can do async and events, so there's even no need for a threadpool or exception swallowing.

But even as it is, for me this code runs over 192.168.0.1..100 range in ~3 seconds, with 10 devices responding and the rest being fucked off by the router before the timeout happens.

If targets are hostnames, one might suspect DNS fuckery. The lookup is probably not counted against the timeout.

Zenith

@Gribnit I think it's actually even slower with thread pooling

Unperverted Vixen

@Zenith I would use Task.Run() instead of explicitly spinning up new Threads myself.

Zenith

@Applied-Mediocrity said in Am I doing non-UI threading right?:

@Zenith I don't like your code ()

Oh come on, I took out my full type resolution and modified Hungarian!

C# Ping

I have half a dozen "ping" methods and only one of them is a real ping. The others are WMI, SQL, and socket connection attempts. That's part of why the structure is a little weird, to handle them all more or less the same way. The table has a few more fields like the exception message (if any).

If targets are hostnames, one might suspect DNS fuckery. The lookup is probably not counted against the timeout.

Possibly. I'm just not sure why it takes so long when threaded. Shouldn't 100 threads with an 8 second timeout take somewhere in the range of 10 seconds instead of 800 seconds?

Zecc

@Zenith said in Am I doing non-UI threading right?:

It's still taking forever, even though the half I can reach returned within about a minute before adding the half I can't reach.

But you're still waiting for all the other threads to join as well before continuing, aren't you?

Zenith

@Zecc But shouldn't they all be running in parallel and starting their 8 second timeouts at more or less the same time? Instead of 8, then 8, then 8, then...

The thread pool version:

using (CountdownEvent Countdown = new CountdownEvent(Targets.Length))
{
  for (Short Pointer = 0; Pointer < Targets.Length; Pointer++)
  {
    Threadpool.QueueUserWorkItem
    {
       try{}catch{}finally{Countdown.Signal();}
    }
  }
  Countdown.Wait();
}

Zecc

@Zenith You're right. I misread what you were saying.

You are of course sure PingAttempt is internally implemented in a way where it can indeed run pings in parallel?

Zenith

@Zecc It's literally:

try
{
  (new ManagementScope("WMI_PATH")).Connect();
  or
  using (Ping Temporary = new Ping("HOST"))
  {
    Temporary.Send();
  }
  or
  using (Socket Temporary = new Socket("HOST", "PORT"))
  {
    Temporary.Connect();
    Temporary.Disconnect();
  }
  true
}
catch
{
  false
}

Threading always makes my head spin in UIs because I have to relearn it every time. I thought it'd be less of a pain without a UI involved.

Zenith

@Zenith I'm not entirely sure what's going on in the framework but the regular ping looks threadsafe (albeit unmanaged). I don't know what Join() actually does because it's an empty extern call.

Zecc

I got nothing. (using LINQPad 7 for testing)

// Using threads
void Main()
{
	string[] Targets = Enumerable.Range(1, 10).Select(i => $"Target {i:00}").ToArray();
	var sw = new Stopwatch();
	var c = new C();
	sw.Start();
	c.Test(Targets);
	$"Finished in {sw.ElapsedMilliseconds} ms".Dump();
}


class C
{
	public void Test(string[] Targets)
	{
		DataTable List = new DataTable();
		List.Columns.Add("Thread", typeof(Thread));
		List.Columns.Add("Response", typeof(Boolean));
		int Index = 0;
		foreach (String Target in Targets)
		{
			Thread WTF = new Thread((object? Index) =>
			{
				try
				{
					PingAttempt(Target, 8);  //8 second timeout
					lock (SyncRoot)
					{
						List.Rows[((int)(Index!))]["Response"] = true;
					}
				}
				catch
				{
					lock (SyncRoot)
					{
						List.Rows[((int)(Index!))]["Response"] = false;
					}
				}
			});
			DataRow Row = List.NewRow();
			Row["Thread"] = WTF;
			Row["Response"] = false;
			List.Rows.Add(Row);
			Index++;
		}
		Index = 0;
		var sw = new Stopwatch();
		sw.Start();
		foreach (DataRow Target in List.Rows)
		{
			((Thread)(Target["Thread"])).Start(Index);
			Index++;
		}
		foreach (DataRow Target in List.Rows)
		{
			((Thread)(Target["Thread"])).Join();
		}
		$"Running tasks took {sw.ElapsedMilliseconds} ms".Dump();

		//List.Dump(collapseTo:1);
	}

	static object SyncRoot = new object();

	static void PingAttempt(string Target, int Timeout)
	{
		$"Pinging {Target}...".Dump();
		Thread.Sleep(TimeSpan.FromSeconds(Timeout));
		$"...Pinged {Target}".Dump();
	}
}

Result:

Pinging Target 01...
Pinging Target 02...
Pinging Target 03...
Pinging Target 04...
Pinging Target 05...
Pinging Target 06...
Pinging Target 07...
Pinging Target 08...
Pinging Target 09...
Pinging Target 10...
...Pinged Target 03
...Pinged Target 01
...Pinged Target 02
...Pinged Target 04
...Pinged Target 05
...Pinged Target 06
...Pinged Target 07
...Pinged Target 08
...Pinged Target 09
...Pinged Target 10
Running tasks took 8069 ms
Finished in 8072 ms

// Using Tasks
void Main()
{
	string[] Targets = Enumerable.Range(1, 10).Select(i => $"Target {i:00}").ToArray();
	var sw = new Stopwatch();
	var c = new C();
	sw.Start();
	c.Test(Targets);
	$"Finished in {sw.ElapsedMilliseconds} ms".Dump();
}


class C
{
	public void Test(string[] Targets)
	{
		DataTable List = new DataTable();
		List.Columns.Add("Thread", typeof(Task));
		List.Columns.Add("Response", typeof(Boolean));
		int Index = 0;
		Task[] Tasks = new Task[Targets.Length];
		foreach (String Target in Targets)
		{
			DataRow Row = List.NewRow();
			int i = Index; // Index will be 10 by the time the tasks run
			Task WTF = new Task(() =>
			{
				$"Task {i} pinging {Target}".Dump();
				try
				{
					PingAttempt(Target, 8);  //8 second timeout
					lock (SyncRoot)
					{
						List.Rows[i]["Response"] = true;
					}
				}
				catch
				{
					lock (SyncRoot)
					{
						List.Rows[i]["Response"] = false;
					}
				}
			});
			Row["Thread"] = Tasks[Index] = WTF;
			Row["Response"] = false;
			List.Rows.Add(Row);
			Index++;
			WTF.Start();
		}
		
		var sw = new Stopwatch();
		sw.Start();
		Task.WhenAll(Tasks).Wait();
		$"Running tasks took {sw.ElapsedMilliseconds} ms".Dump();
		
		//List.Dump(collapseTo:1);
	}

	static object SyncRoot = new object();

	static void PingAttempt(string Target, int Timeout)
	{
		$"Pinging {Target}...".Dump();
		Thread.Sleep(TimeSpan.FromSeconds(Timeout));
		$"...Pinged {Target}".Dump();
	}
}

Result:

Task 1 pinging Target 02
Task 3 pinging Target 04
Task 2 pinging Target 03
Task 0 pinging Target 01
Pinging Target 02...
Pinging Target 03...
Pinging Target 01...
Pinging Target 04...
Task 4 pinging Target 05
Pinging Target 05...
Task 5 pinging Target 06
Pinging Target 06...
Task 6 pinging Target 07
Pinging Target 07...
Task 7 pinging Target 08
Pinging Target 08...
Task 8 pinging Target 09
Pinging Target 09...
Task 9 pinging Target 10
Pinging Target 10...
...Pinged Target 03
...Pinged Target 02
...Pinged Target 01
...Pinged Target 04
...Pinged Target 05
...Pinged Target 06
...Pinged Target 07
...Pinged Target 08
...Pinged Target 09
...Pinged Target 10
Running tasks took 13455 ms
Finished in 13458 ms

Zecc

Just because this was still open this morning, I've let myself be nerd-sniped a little more.

// Using Tasks and PLinq
void Main()
{
	string[] Targets = Enumerable.Range(1, 10).Select(i => $"Target {i:00}").ToArray();
	var sw = new Stopwatch();
	var c = new C();
	sw.Start();
	c.Test(Targets);
	$"Finished in {sw.ElapsedMilliseconds} ms".Dump();
}


class C
{
	public void Test(string[] Targets)
	{
		var sw = new Stopwatch();
		sw.Start();
		Dictionary<String, Task<Boolean>> Results =
			// Calling extension method the weird way
			System.Linq.ParallelEnumerable.AsParallel(Targets)
			.AsOrdered()
			.ToDictionary(Target => Target, async Target =>
			{
				try
				{
					await PingAttemptAsync(Target, 8);  //8 second timeout
					return true;
				}
				catch
				{
					return false;
				}
			});

		DataTable List = new DataTable();
		List.Columns.Add("Target", typeof(string));
		List.Columns.Add("Response", typeof(Boolean));
		foreach(KeyValuePair<String,Task<Boolean>> TargetTaskPair in Results)
		{
			Task<Boolean> Task = TargetTaskPair.Value;
			Boolean Result = Task
				.ConfigureAwait(false)
				.GetAwaiter().GetResult();
			DataRow Row = List.NewRow();
			Row["Target"] = TargetTaskPair.Key;
			Row["Response"] = Result;
			List.Rows.Add(Row);
		}

		$"Running tasks took {sw.ElapsedMilliseconds} ms".Dump();
		
		//List.Dump(collapseTo:1);
	}

	static void PingAttempt(string Target, int Timeout)
	{
		$"Pinging {Target}...".Dump();
		Thread.Sleep(TimeSpan.FromSeconds(Timeout));
		$"...Pinged {Target}".Dump();
	}

	static async Task PingAttemptAsync(string Target, int Timeout)
	{
		$"Pinging {Target}...".Dump();
		await Task.Delay(TimeSpan.FromSeconds(Timeout));
		$"...Pinged {Target}".Dump();
	}
}

Result:

Pinging Target 01...
Pinging Target 02...
Pinging Target 03...
Pinging Target 04...
Pinging Target 05...
Pinging Target 06...
Pinging Target 07...
Pinging Target 08...
Pinging Target 09...
Pinging Target 10...
...Pinged Target 02
...Pinged Target 01
...Pinged Target 03
...Pinged Target 04
...Pinged Target 05
...Pinged Target 08
...Pinged Target 07
...Pinged Target 06
...Pinged Target 09
...Pinged Target 10
Running tasks took 8056 ms
Finished in 8058 ms

I don't know what else to tell you.

Zecc

I bumped the number of targets in each test to 100 and I get this:

Threads: Running tasks took 9013 ms, Finished in 9038 ms
Tasks: Running tasks took 51991 ms, Finished in 51991 ms, and I noticed the starting of tasks seemed staggered (not taking 8 seconds though)
Tasks and PLinq: Running tasks took 8618 ms, Finished in 8619 ms

I thought I was done being nerd-sniped, but know I want to know what's delaying those tasks

Zecc

Tasks are only as asynchronous as their awaits, and I'm not using any, so maybe that's what.

Edit: no, it was because I was first creating the Task, then adding it to the DataTable, and then starting it.

If I change new Task() to Task.Run() it speeds it up to "Running tasks took 48078 ms Finished in 48080 ms". But it still doesn't start them all at the same time. I'm starting to think it's creating all those DataRows that's taking the time.

Edit 2: time's up. I won't be nerd-sniped by this more for now.

Applied Mediocrity

@Zenith said in Am I doing non-UI threading right?:

Oh come on, I took out my full type resolution and modified Hungarian!

Creating 100 function objects at runtime, attached to a Thread that itself lives in the DataTable (so none of it ever gets yeeted by the gc) doesn't look too good.

And DataTable is retarded. Although if you are putting results into DB, okay, whatever.

Still, all this is peanuts. Evidently the problem isn't with threads.

Time the WMI. I'm monitoring a bunch of performance counters. Sometimes - often enough to have been a problem - it takes several seconds to initialize. So now I'm keeping them in global variables instead of opening every time someone requests the numbers. That won't work here, but it can be where the problem is.

Zecc

The example code in Task.WhenAll() docs is interesting:

using System;
using System.Collections.Generic;
using System.Net.NetworkInformation;
using System.Threading;
using System.Threading.Tasks;

public class Example
{
   public static void Main()
   {
      int failed = 0;
      var tasks = new List<Task>();
      String[] urls = { "www.adatum.com", "www.cohovineyard.com",
                        "www.cohowinery.com", "www.northwindtraders.com",
                        "www.contoso.com" };
      
      foreach (var value in urls) {
         var url = value;
         tasks.Add(Task.Run( () => { var png = new Ping();
                                     try {
                                        var reply = png.Send(url);
                                        if (! (reply.Status == IPStatus.Success)) {
                                           Interlocked.Increment(ref failed);
                                           throw new TimeoutException("Unable to reach " + url + ".");
                                        }
                                     }
                                     catch (PingException) {
                                        Interlocked.Increment(ref failed);
                                        throw;
                                     }
                                   }));
      }
      Task t = Task.WhenAll(tasks);
      try {
         t.Wait();
      }
      catch {}   

      if (t.Status == TaskStatus.RanToCompletion)
         Console.WriteLine("All ping attempts succeeded.");
      else if (t.Status == TaskStatus.Faulted)
         Console.WriteLine("{0} ping attempts failed", failed);      
   }
}
// The example displays output like the following:
//       5 ping attempts failed

Gribnit

@Zenith said in Am I doing non-UI threading right?:

@Gribnit I think it's actually even slower with thread pooling

It can be, depends how chokey the pool width is. If the pool was already spun up, could be faster.

Gribnit

@Zecc that looks pretty sane. Wth?

Gribnit

@Applied-Mediocrity said in Am I doing non-UI threading right?:

Time the WMI. I'm monitoring a bunch of performance counters. Sometimes - often enough to have been a problem - it takes several seconds to initialize.

Eh, it's just monitoring...

BernieTheBernie

Neither Task.Run nor Thread.Start do actually start/run the task/thread. These commands just send them to a scheduler which will then decide when to actually start it.

topspin

I have a feeling the problem is with whatever PingAttempt is doing, not with the threading.
Put a line printing the wall clock time before and after that. That should at least tell you if the threads are all starting at the same time and taking longer than expected, or if they’re taking too long to start because something is exhausted.

Do you have CPU load while this is running?

LaoC

I had the same problem last year and I remembered it's a solved problem.

@pinging = `fping --alive --quiet @_`;

cvi

@topspin Indeed - *any* amount of debugging or just basic diagnostic output would likely pinpoint where the problem is.

sockpuppet7

@Zenith said in Am I doing non-UI threading right?:

Shouldn't 100 threads with an 8 second timeout take somewhere in the range of 10 seconds instead of 800

one thing that happens a lot is that the tcp connection is slow, and there is a different way the system do timeouts for that

dkf

@topspin said in Am I doing non-UI threading right?:

I have a feeling the problem is with whatever PingAttempt is doing, not with the threading.

Could be something dumb like a mutex in the guts of the DNS client implementation. Doesn't matter for multi-process use, but with many threads, everything blocks up behind the first thing to have trouble resolving...

Zenith

Well, on me.

My copy protection reads attributes of either a configuration file (IIS) or an assembly (EXE) in a static constructor. When the first thread spins up, it apparently doesn't get the handle I've been using to distinguish between the two (basically an IIS context). Therefore, it falls back to trying to read the assembly attribute, except IIS returns either NULL or a not-CLR assembly so it fails. The failure mode is to exit and that manifests as "the connection was reset." The workaround to keep the copy protection working is to either trigger the constructor(s) before the thread or not use any of my DLL's functions inside a thread.

This is with my original threading code. I don't seem to be able to get that newer signalling version to work at all yet.

Applied Mediocrity

@Zenith said in Am I doing non-UI threading right?:

My copy protection

Zenith

@Applied-Mediocrity said in Am I doing non-UI threading right?:

@Zenith said in Am I doing non-UI threading right?:

My copy protection

Do you want H1Bs lifting my support libraries knowing they're full of code written my way?

Zecc

^You_Wouldn't
^Copy@Zenith

PotatoEngineer

@Zecc said in Am I doing non-UI threading right?:

^You_Wouldn't
^Copy@Zenith

No, but I'd print him!

topspin

@Zenith said in Am I doing non-UI threading right?:

@Applied-Mediocrity said in Am I doing non-UI threading right?:

@Zenith said in Am I doing non-UI threading right?:

My copy protection

Do you want H1Bs lifting my support libraries knowing they're full of code written my way?

Are you releasing this somewhere? It sounded like you’re running into this trouble with your own code, not something you’re deploying at a client’s.

Zenith

@topspin Grey area. It's my library code adapted for a work project. Employer's H1Bs love to steal code from employer (and others) and use on other projects. Plus, my deep-seated loathing of H1Bs. So no I don't want their grubby mitts anywhere near code too brillant for their microbrains.

Zenith

@Zecc said in Am I doing non-UI threading right?:

^You_Wouldn't
^Copy @Zenith

You shouldn't copy @Zenith?

I can't believe we don't have a Roberto stabbing icon!

Applied Mediocrity

@Zenith said in Am I doing non-UI threading right?:

Do you want H1Bs lifting my support libraries knowing they're full of code written my way?

It's that you fell for one of the classic blunders: your DRM interferes with correct operation of your own program

dkf

@Zenith said in Am I doing non-UI threading right?:

So no I don't want their grubby mitts anywhere near code too brillant for their microbrains.

I remember a friend complaining about that. His solution was to make the code even more brilliant, such that you'd need an advanced degree in mathematics to understand what it did and why it was so screaming fast. I know it involved moving something to do with non-standard fourier transforms onto a GPU so that they could be used to do automatic image registration in real time despite the distortions in the equipment acquiring the images, but it definitely was above my knowledge level...

The upshot was his code caused everyone who interacted with it to go "How the hell does that even work?!"

BernieTheBernie

@dkf said in Am I doing non-UI threading right?:

"How the hell does that even work?!"

So, their reaction was not at all different from their reaction to any other code copied from StackOverflow.

Gribnit

@BernieTheBernie I have some code I even wrote where the adverbs are a bit subtle in operation - too far from the apparent verb.

dkf

@BernieTheBernie said in Am I doing non-UI threading right?:

@dkf said in Am I doing non-UI threading right?:

"How the hell does that even work?!"

So, their reaction was not at all different from their reaction to any other code copied from StackOverflow.

There is a difference between stuff that can be puzzled out if you have a quiet half hour with no meeting to interrupt you, and high wizardry that needs a couple of advanced degrees and a generous chunk of domain knowledge to comprehend.

HardwareGeek

@dkf There is also a difference between and .

dcon

@dkf said in Am I doing non-UI threading right?:

stuff that can be puzzled out if you have a quiet half hour with ~~no meeting~~nothing to interrupt you,

Because you know someone will walk past you and ask a question. And, no, it's no different if you're working at home.

dkf

@HardwareGeek said in Am I doing non-UI threading right?:

@dkf There is also a difference between and .

Just a

dkf

@dcon said in Am I doing non-UI threading right?:

Because you know someone will walk past you and ~~ask a question~~give you a coffee. And, no, it's no different if you're working at home.

It definitely is different!

BernieTheBernie

@dkf said in Am I doing non-UI threading right?:

@BernieTheBernie said in Am I doing non-UI threading right?:

@dkf said in Am I doing non-UI threading right?:

"How the hell does that even work?!"

So, their reaction was not at all different from their reaction to any other code copied from StackOverflow.

There is a difference between stuff that can be puzzled out if you have a quiet half hour with no meeting to interrupt you, and high wizardry that needs a couple of advanced degrees and a generous chunk of domain knowledge to comprehend.

That difference does not exist with my cow-orkers, and I am sure for most duhveloppers: everything is beyond their grasp.

Arantor

@BernieTheBernie I find the worst kind of duhveloper are the self-taught kind who have been doing it just enough to figure out how to get by, where everything will be a sub-optimal mess and anything beyond their (often quite limited) knowledge is simply impossibru.

dcon

@Arantor said in Am I doing non-UI threading right?:

@BernieTheBernie I find the worst kind of duhveloper are the self-taught kind who have been doing it just enough to figure out how to get by, where everything will be a sub-optimal mess and anything beyond their (often quite limited) knowledge is simply impossibru.

Don't forget, they're typically the boss's insert any family relationship too.

Applied Mediocrity

@Arantor ...

Okay, not exactly self-taught.

Arantor

@Applied-Mediocrity that's the thing - this kind are always self taught. They just hit a point where they figure they know some definition of 'everything' and stop learning, to the point where they glass-ceiling themselves.

I've yet to meet anyone who studied CS at a university fall into the same trap. They fall into other, different traps.

Applied Mediocrity

@Arantor said in Am I doing non-UI threading right?:

I've yet to meet anyone who studied CS at a university fall into the same trap

No, seriously. I've mostly stopped learning. I've... well, that's for Lounge, if anything.

Arantor

@Applied-Mediocrity That's not what I mean, though.

You might have stopped learning - but you don't assume that, as a consequence, that you know everything, and that by extension what you don't know is by definition impossible.

That is the trap self-taught people seem to fall into.