We’re rewarding the question askers & reputations are being recalculated! Read more.
24

The reason why there is no async API for a dictionary is, that all operations on a dictionary are so fast, that there is no need for asynchronicity. For concurrent scenarios there is the thread safe variant - the ConcurrentDictionary. Adding an async API to these dictionaries has absolutely zero value. Rather it increases complexity and reduces performance....


20

I would advise against this approach, you are correct in the fact that it is simple, however it's not foolproof. There is nothing actually ensuring that someone doesn't just call .Add() on the collection bypassing your extension method which allows for difficult to find bugs/race conditions. Don't forget that all Monitor.Enter does is ensure that if ...


18

Short version: it looks like you're just trying to get a per-type ObjectCache. That's a basic singleton pattern, so I'm unsure what you're trying to gain through all this extra cruft in the class. If it were me, I would consider the following code: public static class GlobalTypedObjectCache<T> { public static ObjectCache Cache { get; private set; }...


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

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 ...


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

First of all, your locking seems ok, I don't expect any drawbacks by using multiple threads. That being said, let us review this from bottom to top. public enum Tryb { FILE = 0, CONSOLE = 1, FILEANDCONSOLE = 2 } The name of that enum is at the nicest say unclear. A name like LogTarget would be more clear and its meaning would be obvious ...


15

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....


13

Style/Readability Unnecessary XML comments. For example: /// <summary> /// Gets the failure count. /// </summary> /// <value> /// The failure count. /// </value> public int FailCount { get { return _failCount; } } The summary and value not only repeat each other, but also repeat the property name. Statement in comments of what's ...


13

General Stuff Would be nice to have inline documentation (///) on classes and public members, but everything is pretty simple and understandable. Some might question the name _hashSet, which tells you nothing about it's purpose, but it's private so not a massive concern. You've correctly identified that you can use a ReaderWriterLock to control access, as ...


13

Threading Design Your implementation has a very intrusive lock for all read and write operations, using the SemaphoreSlim with max concurrency 1. try { await _semaphoreSlim.WaitAsync()// <- both read/write operations acquire single mutex return await Task.Run(async () => { return await func(_dictionary, keyValuePair); }); } ...


12

You are guarding the state of one variable std::queue<unsigned char*> _queue_; So you only have one mutex. pthread_mutex_t push_mutex; pthread_mutex_t pop_mutex; If you have two then push and pop can modify queue at the same time. Make sure your mutex locking is exception safe by using RAII. Note all operations were you do start operation stop ...


12

First off, never lock on this. You never know who else is locking on that instance. Conversely, clients don't know that the cache is locking on itself either. For example, this seemingly innocent code would lead to a deadlock: //Thread A lock(cache) lock(someObject) { //do something } //Thread B lock(someObject) { //here, the cache will lock on ...


12

Please note that the code should be presented in the same way that you use it to avoid spurious errors. Compiling your code as is, gave me an error on the specialization of submit. Naming pair_t is a rather generic name that does not tell what is stored m_work could be improved a little although I don't have any idea yet m_mutex should be renamed to ...


12

struct Fork { std::mutex mux; }; std::vector<Fork*> forks; I think the problem you want to solve by using pointers here is that a std::mutex is not copyable nor movable. By using raw owning pointers, you'll get a bunch of unpleasant properties. For example, you have to implement a destructor, and might have to worry about exception safety. ...


12

I don't think Log should be public, it is confusing. Me, as a client of your logging API, shouldn't need to create an instance of TextWriter to be able to log something. If I want to do some IO, I'll do it all by myself and won't pass by the logging API. The naming is a little flawed. tw should be (I think), logTextWriter or something like that. Good ...


12

Please correct me if I am wrong, but isn't such scenario possible with accepted answer? public T Swap() { var swappedBuffer = Interlocked.Exchange(ref _current, Next); // 1 Next = swappedBuffer; // 2 return swappedBuffer; // 3 } And it goes like that: (A and B denotes some object of type T): _current = A Next = B Thread 1 executes Line 1: ...


12

Naming There is one 'u' in ConcuurentCircularBuffer that wants to be a 'r' --> ConcurrentCircularBuffer There is no need to abbreviate name like size (sz --> size) I would call last to something more descriptive like lastIndex. Code Style sz, buffer and lockObject should be read-only. There is no need to skip the first array element, just start with 0. ...


12

Dinning Philosophers How should I improve my solution for Dining Philosopher's problem? Well first implementing the Dining Philosophers problem would be good. This is an interesting problem you implemented but its not the classic Dinning Philosophers The issue here is that in your implementation each philosopher tries to grab any two forks. In the ...


12

In terms of the basic thread locking, it looks like it is doing the right thing, but there are a number of issues in how you are calculating the account balance, and also some escaped locking as well. Note, your post is titled "Synchronized implementation", but it is not, it is a locked implementation. Synchronization is different, and, in this case, it may ...


11

Think I've found one concurrency issue : in private void extendStateTo(final PrimeState estate, final int to) { if (lock.compareAndSet(false, true)) { // we are the thread with the lock... try { // OK, we are the only thread in here, and the state is not good // enough... // create a new state that ...


11

It's pretty hard to break something that uses a global lock around everything. So this seems pretty thread-safe. But that doesn't answer the question of why you'd want to use this. Asynchronous calls are useful for things that take a long time, particularly if you can delegate the "waiting" to some low-level event based solution (HTTP requests for example)....


10

Late answer to this question..... There are a few things which should be considered when implementing a solution like this, and best-practice comes in to play here. One nit-pick ... before we go further, your interface the methods executeAsynchronous and executeSynchronous, whereas your implementation has executeAsync and execute. This is a ...


10

Don't right align your comments that way. It makes it near impossible to read the code and the comment at the same time. They're useless out there. I personally don't mind end of line comments, but right aligned... no. Others may disagree with me though. To clarify, this is hard to read: delete receivedMessage[id]; ...


10

There are a few items to make this code better. Naming. Once is an OK name for the class, but the method name IsDone is a problem. This is an 'atomic' operation that sets values, as well as gets values. A method called something like "Trigger", and changing the class name to a common term like OneShot, will give you the semantics like: private readonly ...


10

I have only a few comments: The name you gave to sleep_for_random_time() is ambiguous. Some might interpret that the function sleeps for a random amount of time, while some might interpret that it returns a random value for a call to sleep_for(). The latter is true. Either change the name or change the functionality. Since it is a local function, then ...


10

In addition to @JanDotNet's answer: You can use "less strict" lock in your code, specifically ReaderWriterLock or ReaderWriterLockSlim. As I see from your code, you can execute Read in parallel threads but cannot execute Put and Read in parallel. These classes are exactly for this case and will improve overall performance.


10

volatile bool thread_shutdown = false; This is definitely not correct. volatile is not for thread safety. What you want here is atomic<bool>. Other than that, I'd say everything else looks technically kosher. The thing is, the semantics of your move operations will be weird now. For example, if I write: auto func(A a) {} auto a = A{}; func(std::...


10

Let's start with the good: Ohh recent Python 3, nice! Good documentation Good use of typing. This is great for datastructures. Good formatting of your code. Nice spacing. Decent naming Good use of defaultdict Good use of asserts Things I noticed along the way: contains is usually spelled __contains__. This way you can do item in queue. I'd rename ...


10

Task task; while (m_enabled) { { std::unique_lock<std::mutex> lg{ m_mu }; while (m_tasks.empty() && m_enabled) m_cond.wait(lg); } if (!m_tasks.empty()) {// there is a task available std::unique_lock<std::mutex> lg{ m_mu }; task = std::move(m_tasks.front()); m_tasks.pop(); ...


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