02077ffc42
Before 2019, the entire ConditionVariable system was "giant"-locked: that is, there was a single global lock that all ConditionVariable and ConditionVariableEntry operations had to pass through. This of course was not very performant on multicore systems and when ConditionVariables see significant use, so I reworked it then to have more granular locking. Those patches took a number of attempts to get right, as having two objects in separate threads that can each access the other not turn into a deadlock or use-after-free is not easy to say the least, and the ultimate solution I came up with erased most of the performance gains I initially saw on the first (partially broken) patchsets. So I have wanted to revisit this and see if there was a better way even since then. Recently there have been a few reports of ConditionVariable-related panics (apparently double unlocks), notably #16894, and so that was reason enough to actually revisit this code and see if a better solution could be found. Well, I think I have come up with one: after this commit, Entries no longer have their own lock, and instead accesses to Entry members are almost always atomic; and there is now a case where we spin inside Variable::_NotifyLocked as well as one in Entry::_RemoveFromVariable. This leads to somewhat simpler code (no more lock/unlock dance in Notify), though it is significantly more difficult to understand the nuances of it, so I have left a sizable number of comments explaining the intricacies of the new logic. Note: I initially tried 1000 for "tries", but on a few instances I did see the panic hit, strangely. I don't think the code that is waited on can be reasonably reduced any further, so I have just increased the limit to 10000 (which is still well below what spinlocks use.) Hopefully this suffices. Quick benchmark, x86, compiling HaikuDepot and the mime_db in VMware, 2 cores: before: real 0m23.627s user 0m25.152s sys 0m7.319s after: real 0m23.962s user 0m25.229s sys 0m7.330s Though I occasionally I saw sys times as low as 7.171s, so this seems to be at least not a regression if not a definitive improvement. Change-Id: Id042947976885cd5c1433cc4290bdf41b01ed10e Reviewed-on: https://review.haiku-os.org/c/haiku/+/4727 Tested-by: Commit checker robot <no-reply+buildbot@haiku-os.org> Reviewed-by: Alex von Gluck IV <kallisti5@unixzen.com>
127 lines
2.8 KiB
C++
127 lines
2.8 KiB
C++
/*
|
|
* Copyright 2007-2011, Ingo Weinhold, ingo_weinhold@gmx.de.
|
|
* Copyright 2019, Haiku, Inc. All rights reserved.
|
|
* Distributed under the terms of the MIT License.
|
|
*/
|
|
#ifndef _KERNEL_CONDITION_VARIABLE_H
|
|
#define _KERNEL_CONDITION_VARIABLE_H
|
|
|
|
|
|
#include <OS.h>
|
|
|
|
#include <debug.h>
|
|
|
|
#ifdef __cplusplus
|
|
|
|
#include <util/DoublyLinkedList.h>
|
|
#include <util/OpenHashTable.h>
|
|
|
|
|
|
struct ConditionVariable;
|
|
|
|
|
|
struct ConditionVariableEntry
|
|
: DoublyLinkedListLinkImpl<ConditionVariableEntry> {
|
|
public:
|
|
ConditionVariableEntry();
|
|
~ConditionVariableEntry();
|
|
|
|
bool Add(const void* object);
|
|
status_t Wait(uint32 flags = 0, bigtime_t timeout = 0);
|
|
status_t Wait(const void* object, uint32 flags = 0,
|
|
bigtime_t timeout = 0);
|
|
|
|
inline status_t WaitStatus() const { return fWaitStatus; }
|
|
|
|
inline ConditionVariable* Variable() const { return fVariable; }
|
|
|
|
private:
|
|
inline void _AddToLockedVariable(ConditionVariable* variable);
|
|
void _RemoveFromVariable();
|
|
|
|
private:
|
|
ConditionVariable* fVariable;
|
|
Thread* fThread;
|
|
status_t fWaitStatus;
|
|
|
|
friend struct ConditionVariable;
|
|
};
|
|
|
|
|
|
struct ConditionVariable {
|
|
public:
|
|
void Init(const void* object,
|
|
const char* objectType);
|
|
// for anonymous (unpublished) cvars
|
|
|
|
void Publish(const void* object,
|
|
const char* objectType);
|
|
void Unpublish();
|
|
|
|
inline void NotifyOne(status_t result = B_OK);
|
|
inline void NotifyAll(status_t result = B_OK);
|
|
|
|
static void NotifyOne(const void* object, status_t result);
|
|
static void NotifyAll(const void* object, status_t result);
|
|
// (both methods) caller must ensure that
|
|
// the variable is not unpublished
|
|
// concurrently
|
|
|
|
void Add(ConditionVariableEntry* entry);
|
|
|
|
status_t Wait(uint32 flags = 0, bigtime_t timeout = 0);
|
|
// all-in one, i.e. doesn't need a
|
|
// ConditionVariableEntry
|
|
|
|
const void* Object() const { return fObject; }
|
|
const char* ObjectType() const { return fObjectType; }
|
|
|
|
static void ListAll();
|
|
void Dump() const;
|
|
|
|
private:
|
|
void _Notify(bool all, status_t result);
|
|
void _NotifyLocked(bool all, status_t result);
|
|
|
|
protected:
|
|
typedef DoublyLinkedList<ConditionVariableEntry> EntryList;
|
|
|
|
const void* fObject;
|
|
const char* fObjectType;
|
|
|
|
spinlock fLock;
|
|
EntryList fEntries;
|
|
int32 fEntriesCount;
|
|
|
|
ConditionVariable* fNext;
|
|
|
|
friend struct ConditionVariableEntry;
|
|
friend struct ConditionVariableHashDefinition;
|
|
};
|
|
|
|
|
|
inline void
|
|
ConditionVariable::NotifyOne(status_t result)
|
|
{
|
|
_Notify(false, result);
|
|
}
|
|
|
|
|
|
inline void
|
|
ConditionVariable::NotifyAll(status_t result)
|
|
{
|
|
_Notify(true, result);
|
|
}
|
|
|
|
|
|
extern "C" {
|
|
#endif // __cplusplus
|
|
|
|
extern void condition_variable_init();
|
|
|
|
#ifdef __cplusplus
|
|
} // extern "C"
|
|
#endif
|
|
|
|
#endif /* _KERNEL_CONDITION_VARIABLE_H */
|