143

Note: At some point, this review drifted into the realm of assembler and GMP. An actual review is at the end of this post, whereas the first section discusses the runtime-problems concerning pow, wrong data types and arbitrary large integers. No life time for run time Would there be any way (on my current machine) to get this to run in my lifetime? ...


91

Rather than reviewing your specific code -- which at first glance seems very, very buggy -- let me answer your question directly: What are the best practices with multithreading in C#? Don't. Multithreading is a bad idea. Programs are hard enough to understand already with a single point of control flow in them; why on earth would you want to add a ...


76

Well let's start with this: for (a = 1; a < 100000; a++) { for (b = 1; b < 300000; b++) { for (c = 1; c < 500000; c++) { let's ignore d for now. What do you do here? You check 1, 1, 1, then 1, 1, 2, then 1, 1, 3, ... up to 1, 1, 499999. Then you start over at 1, 2, 1. But you already checked 1, 1, 2, so why are you checking 1, 2, 1? ...


39

Funny how the universe works - I just finished my own implementation of a thread pool (albeit in C++17), and it looks a lot like yours. I found this question on the front page when I went to post my own - here's hoping we're both on the right track! Mark the copy c'tor and operator= as deleted Instead of actually implementing something that you don't want to ...


31

All of the answers given (when this 'answer' was first written) ignore an important constraint: native types. In C, a long int is a 32-bit (or greater) signed type, meaning the largest positive value that can be (counted upon to be) represented is \$2^{31}-1\$. The largest possible input to a function which calculates a fourth-power and [[can be counted on ...


31

I'm not able to comment on vnp's solution, but vnp was right the first time: you can brute force it in \$O(n^2\log(n))\$ time and \$O(n)\$ space. You don't need \$O(n^2)\$ space because you don't have to store the whole list of \$a^4+b^4\$ or \$d^4-c^4\$ upfront. Instead you only need to be able to list the values of \$a^4+b^4\$ and \$d^4-c^4\$ in ascending ...


30

Some interesting things no one have mentioned about this, but it is an improvement when looking for the smallest solution: If the \$\gcd(a,b,c) \neq 1\$ then \$a^4\$,\$b^4\$,\$c^4\$ and \$d^4\$ are all divisible by that gcd to the 4th power, giving a smaller solution. Therefore, at least one of \$a\$,\$b\$, and \$c\$ must be odd, since if they are all even ...


23

As you are correctly assuming there are multiple threading related issues with your code, but lets tease you and start with the usual suspects. Naming The name apply_prime is misleading and inexpressive. Neither does the function really require a prime nor does apply do it justice. You should name it something along the lines: strike_out_multiples or ...


22

Firstly, it's a good first attempt at writing some threaded code. The major sticking point is that you're passing in an int & and returning void. Of course, std::thread will just run some code and won't return you a result. However, within the C++11 threading library, there are a number of things you can use that will allow you to return results, instead ...


22

Generally it's nice, well-structured code, but it relies on a promise that might not be kept. Specifically, simultaneous access to different elements in std::vector<bool> is not guaranteed to be thread-safe because storage bytes may be shared by multiple bits in the vector. Consider an alternative way to slice things. Each thread could be ...


22

Read the source Since you've already identified the author, why not read the original paper? Some misconceptions In fact, the numbers found by Elkies were not the ones cited in the question. They were found by Roger Frye using a computer. How did he do that back in 1988? It's mentioned in the paper: Postscript. While our first counterexample ...


20

Well you can optimize FileContains very simply - stop calling Text.ToLower() on every iteration! In fact, at that point it's really simple to rewrite. Here's the code for the original question, which didn't have the Substring part: public static bool FileContains(string text) { // Variable name changed to save my eyes from the horror. return lines....


20

Your new approach is much better than the previous one. Instead of deriving from List and using new to hide the base class implementations you create a wrapper around a List implementing IList - this is a much more robust way. However there are a few oddities: You don't need to capture the result of a function call in a local variable to return it outside ...


19

The second one is much easier to read, it expresses the developers intent. At first sight I (as a maintainer or another developer in the same team, for example) just want a quick overview about the code and don't care about the details. (How the code creates a server socket, for example.) The second one exactly gives that. The System.exit side effect could ...


18

I'm not sure why this is async. Unless you have a good reason for write to the file asynchronously, this is wasted effort. If you lock properly, this will work fine. Your locker variable should be marked static. You can then get away with something like this: using System; using System.IO; using System.Text; using System.Threading; using System....


18

I recommend to rewrite a problem as finding \$a,b,c,d\$ such that \$a^4 + b^4 = d^4 - c^4\$. Now you may only operate in pairs of powers. Building a table of sums takes \$O(n^2)\$. Building the table of differences also takes \$O(n^2)\$. Sorting them takes \$O(n^2\log{n})\$. Matching sorted tables takes \$O(n^2)\$ in a linear merge-like scan. Overall ...


18

Multiple threads are usually used to compute things in parallel. In this example nothing is computed in parallel: while one thread is running, the other is waiting. With no practical value, it's not a great demonstration of multithreading. I suggest to look for more practical targets in the future. Since evenready and oddready always have opposite values, ...


17

You can use Parallel class: private static void ExportDocTypes(IEnumerable<string> docTypes) { int i = -1; Parallel.ForEach(docTypes, docType => ExportDocuments(docType, Interlocked.Increment(ref i))); } http://msdn.microsoft.com/en-us/library/system.threading.tasks.parallel_methods%28v=vs.110%29.aspx


17

There's also a generalization problem. I agree! Whenever you start writing things like: f0 = Floor(0) f1 = Floor(1) f2 = Floor(2) f3 = Floor(3) f4 = Floor(4) f5 = Floor(5) f6 = Floor(6) f7 = Floor(7) f8 = Floor(8) stop immediately, and consider using lists and/or dictionaries instead: floors = [Floor(i) for i in range(9)] This now makes subsequent ...


16

Ignoring the memory problems with cache invalidation (which will slow the code down). Creating a thread is relatively expensive (as you have to allocate a stack and maintain it). So rather than creating and destroying threads it is better to maintain a thread pool and reuse the threads. The number of threads to put in the pool should be slightly larger ...


16

Easy Stuff you already know. Probably part of your automated scripts to build new files. #pragma once But for new comers I would point out the more standard include guards are compatible everywhere. Don't be lazy std::lock_guard<xtd::fast_recursive_mutex> _(m_mutex); Though not technically wrong as an identifier (_). How many people do ...


16

I really don't get why people are so afraid to create extra classes. 200+ lines in one Program.cs file isn't something I consider good style. Sure, this is just a project for fun, but even then... When I write a small program for myself, often they are console applications. And the first thing I do is create a class that will contain all of the logic etc. ...


16

In your chatGUI's __init__ your setting up some constants, that should really just be defined as constants at the class level: class chatGUI: #Class That Handles GUI related tasks. BGCOLOUR = "#2a2a2a" # First Colour was: "#607D8B" # Contains hyperlink triggers. HYPER_LIST = ("http://", "www.", "https://", "ftp://") WINDOW_TITLE = "Oke's ...


16

I'm mostly going to play advocate for the devil here. Lock-free doesn't mean fast There is a rather persistent misconception that lock-free algorithms are faster than locking algorithms. However, that may not be true. Modern mutex implementations are extemely fast in the uncontended case, and when there is a lot of contention they use a system call that lets ...


15

Not exactly a review, but since you are asking for simpler way... The simpler way is to run your tasks via dispatcher. Just run it on background thread: _thread = new Thread(() => { _dispatcher = Dispatcher.CurrentDispatcher; Dispatcher.Run(); }); _thread.Start(); And use _dispatcher....


14

Tasks can have results, you should take advantage of that, instead of assigning a local variable from a lambda. And WaitAny() returns the index of the Task that finished first. This means you can do something like: public static bool IsPrimeHybrid(BigInteger number) { var byPureCalc = Task.Run(() => IsPrimeNaive(number)); var bySearchFirst = Task....


14

Interesting idea and well-done! Naming I really don't like the names. The names like clsMultiThread is somewhat misleading, since as you noted they don't actually provide any true multi-threading. A unwary user would expect it to work with anything and would be disappointed when all of their queued works painfully completes synchronously. ;) Also, we are ...


14

I see some things that may help you improve your program. Don't abuse using namespace std Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. Eliminate global variables where practical The code declares and uses 5 global variables. Global variables obfuscate the actual dependencies within code and make ...


14

@osuka_ and @anderas gave some very good advice. I just have a couple things I want to add: Macros #define MAX_THREADS std::thread::hardware_concurrency() - 1; While it doesn't really make sense to do arithmetic on this, macros like this should be surrounded by parentheses so the order of operations works correctly. Without it you get weird stuff like (...


Only top voted, non community-wiki answers of a minimum length are eligible