Commit b03ce831 by Ben Clayton

Reimplement vk::Query using sw sync primitives.

Hide all the error-prone synchronization logic in the class, and expose a documented API. This should have no impact on behavior - it was authored to make it harder to break things in the future. That said, it appears to fix a whole bunch of flakes with the dEQP query tests. Bug: b/133127573 Change-Id: I5c30b79b9b1cd36dba1fa2d3c34af0f5bd62772a Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/31816 Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Reviewed-by: 's avatarChris Forbes <chrisforbes@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com> Tested-by: 's avatarBen Clayton <bclayton@google.com>
parent e2cb4e05
...@@ -289,7 +289,7 @@ namespace sw ...@@ -289,7 +289,7 @@ namespace sw
{ {
for(auto query : queries) for(auto query : queries)
{ {
if(query->type == type) if(query->getType() == type)
{ {
return true; return true;
} }
...@@ -377,7 +377,7 @@ namespace sw ...@@ -377,7 +377,7 @@ namespace sw
draw->queries = new std::list<vk::Query*>(); draw->queries = new std::list<vk::Query*>();
for(auto &query : queries) for(auto &query : queries)
{ {
++query->reference; // Atomic query->start();
draw->queries->push_back(query); draw->queries->push_back(query);
} }
} }
...@@ -855,34 +855,23 @@ namespace sw ...@@ -855,34 +855,23 @@ namespace sw
{ {
for(auto &query : *(draw.queries)) for(auto &query : *(draw.queries))
{ {
std::unique_lock<std::mutex> mutexLock(query->mutex); switch(query->getType())
switch(query->type)
{ {
case VK_QUERY_TYPE_OCCLUSION: case VK_QUERY_TYPE_OCCLUSION:
for(int cluster = 0; cluster < clusterCount; cluster++) for(int cluster = 0; cluster < clusterCount; cluster++)
{ {
query->data += data.occlusion[cluster]; query->add(data.occlusion[cluster]);
} }
break; break;
default: default:
break; break;
} }
int queryRef = --query->reference; // Atomic query->finish();
if(queryRef == 0)
{
query->state = vk::Query::FINISHED;
}
// Manual unlocking is done before notifying, to avoid
// waking up the waiting thread only to block again
mutexLock.unlock();
query->condition.notify_one();
} }
delete draw.queries; delete draw.queries;
draw.queries = 0; draw.queries = nullptr;
} }
draw.vertexRoutine->unbind(); draw.vertexRoutine->unbind();
......
...@@ -32,7 +32,7 @@ ...@@ -32,7 +32,7 @@
namespace vk namespace vk
{ {
class DescriptorSet; class DescriptorSet;
struct Query; class Query;
} }
namespace sw namespace sw
......
...@@ -13,7 +13,6 @@ ...@@ -13,7 +13,6 @@
// limitations under the License. // limitations under the License.
#include "VkQueryPool.hpp" #include "VkQueryPool.hpp"
#include "Common/Thread.hpp"
#include <chrono> #include <chrono>
#include <cstring> #include <cstring>
...@@ -21,6 +20,69 @@ ...@@ -21,6 +20,69 @@
namespace vk namespace vk
{ {
Query::Query() : finished(sw::Event::ClearMode::Manual), state(UNAVAILABLE), type(INVALID_TYPE), value(0) {}
void Query::reset()
{
ASSERT(wg.count() == 0);
finished.clear();
auto prevState = state.exchange(UNAVAILABLE);
ASSERT(prevState != ACTIVE);
type = INVALID_TYPE;
value = 0;
}
void Query::prepare(VkQueryType ty)
{
auto prevState = state.exchange(ACTIVE);
ASSERT(prevState == UNAVAILABLE);
type = ty;
}
void Query::start()
{
ASSERT(state == ACTIVE);
wg.add();
}
void Query::finish()
{
if (wg.done())
{
auto prevState = state.exchange(FINISHED);
ASSERT(prevState == ACTIVE);
finished.signal();
}
}
Query::Data Query::getData() const
{
Data out;
out.state = state;
out.value = value;
return out;
}
VkQueryType Query::getType() const
{
return type;
}
void Query::wait()
{
finished.wait();
}
void Query::set(int64_t v)
{
value = v;
}
void Query::add(int64_t v)
{
value += v;
}
QueryPool::QueryPool(const VkQueryPoolCreateInfo* pCreateInfo, void* mem) : QueryPool::QueryPool(const VkQueryPoolCreateInfo* pCreateInfo, void* mem) :
pool(reinterpret_cast<Query*>(mem)), type(pCreateInfo->queryType), pool(reinterpret_cast<Query*>(mem)), type(pCreateInfo->queryType),
count(pCreateInfo->queryCount) count(pCreateInfo->queryCount)
...@@ -61,7 +123,7 @@ namespace vk ...@@ -61,7 +123,7 @@ namespace vk
// The sum of firstQuery and queryCount must be less than or equal to the number of queries // The sum of firstQuery and queryCount must be less than or equal to the number of queries
ASSERT((firstQuery + queryCount) <= count); ASSERT((firstQuery + queryCount) <= count);
VkResult result = VK_SUCCESS; VkResult result = VK_SUCCESS;
uint8_t* data = static_cast<uint8_t*>(pData); uint8_t* data = static_cast<uint8_t*>(pData);
for(uint32_t i = firstQuery; i < (firstQuery + queryCount); i++, data += stride) for(uint32_t i = firstQuery; i < (firstQuery + queryCount); i++, data += stride)
...@@ -72,14 +134,16 @@ namespace vk ...@@ -72,14 +134,16 @@ namespace vk
// VK_NOT_READY. However, availability state is still written to pData for those // VK_NOT_READY. However, availability state is still written to pData for those
// queries if VK_QUERY_RESULT_WITH_AVAILABILITY_BIT is set. // queries if VK_QUERY_RESULT_WITH_AVAILABILITY_BIT is set.
auto &query = pool[i]; auto &query = pool[i];
std::unique_lock<std::mutex> mutexLock(query.mutex);
if(flags & VK_QUERY_RESULT_WAIT_BIT) // Must wait for query to finish if(flags & VK_QUERY_RESULT_WAIT_BIT) // Must wait for query to finish
{ {
query.condition.wait(mutexLock, [&query] { return query.state == Query::FINISHED; }); query.wait();
} }
const auto current = query.getData();
bool writeResult = true; bool writeResult = true;
if(pool[i].state == Query::ACTIVE) if(current.state == Query::ACTIVE)
{ {
result = VK_NOT_READY; result = VK_NOT_READY;
writeResult = (flags & VK_QUERY_RESULT_PARTIAL_BIT); // Allow writing partial results writeResult = (flags & VK_QUERY_RESULT_PARTIAL_BIT); // Allow writing partial results
...@@ -90,11 +154,11 @@ namespace vk ...@@ -90,11 +154,11 @@ namespace vk
uint64_t* result64 = reinterpret_cast<uint64_t*>(data); uint64_t* result64 = reinterpret_cast<uint64_t*>(data);
if(writeResult) if(writeResult)
{ {
result64[0] = pool[i].data; result64[0] = current.value;
} }
if(flags & VK_QUERY_RESULT_WITH_AVAILABILITY_BIT) // Output query availablity if(flags & VK_QUERY_RESULT_WITH_AVAILABILITY_BIT) // Output query availablity
{ {
result64[1] = pool[i].state; result64[1] = current.state;
} }
} }
else else
...@@ -102,11 +166,11 @@ namespace vk ...@@ -102,11 +166,11 @@ namespace vk
uint32_t* result32 = reinterpret_cast<uint32_t*>(data); uint32_t* result32 = reinterpret_cast<uint32_t*>(data);
if(writeResult) if(writeResult)
{ {
result32[0] = static_cast<uint32_t>(pool[i].data); result32[0] = static_cast<uint32_t>(current.value);
} }
if(flags & VK_QUERY_RESULT_WITH_AVAILABILITY_BIT) // Output query availablity if(flags & VK_QUERY_RESULT_WITH_AVAILABILITY_BIT) // Output query availablity
{ {
result32[1] = pool[i].state; result32[1] = current.state;
} }
} }
} }
...@@ -123,31 +187,14 @@ namespace vk ...@@ -123,31 +187,14 @@ namespace vk
UNIMPLEMENTED("flags"); UNIMPLEMENTED("flags");
} }
std::unique_lock<std::mutex> lock(pool[query].mutex); pool[query].prepare(type);
ASSERT(pool[query].state == Query::UNAVAILABLE); pool[query].start();
pool[query].state = Query::ACTIVE;
pool[query].data = 0;
pool[query].reference = 1;
pool[query].type = type;
} }
void QueryPool::end(uint32_t query) void QueryPool::end(uint32_t query)
{ {
ASSERT(query < count); ASSERT(query < count);
pool[query].finish();
#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
{
std::unique_lock<std::mutex> mutexLock(pool[query].mutex);
ASSERT(pool[query].state == Query::ACTIVE);
}
#endif
int ref = --pool[query].reference;
if(ref == 0)
{
std::unique_lock<std::mutex> mutexLock(pool[query].mutex);
pool[query].state = Query::FINISHED;
}
} }
void QueryPool::reset(uint32_t firstQuery, uint32_t queryCount) void QueryPool::reset(uint32_t firstQuery, uint32_t queryCount)
...@@ -157,10 +204,7 @@ namespace vk ...@@ -157,10 +204,7 @@ namespace vk
for(uint32_t i = firstQuery; i < (firstQuery + queryCount); i++) for(uint32_t i = firstQuery; i < (firstQuery + queryCount); i++)
{ {
std::unique_lock<std::mutex> mutexLock(pool[i].mutex); pool[i].reset();
pool[i].state = Query::UNAVAILABLE;
pool[i].data = 0;
} }
} }
...@@ -169,8 +213,7 @@ namespace vk ...@@ -169,8 +213,7 @@ namespace vk
ASSERT(query < count); ASSERT(query < count);
ASSERT(type == VK_QUERY_TYPE_TIMESTAMP); ASSERT(type == VK_QUERY_TYPE_TIMESTAMP);
std::unique_lock<std::mutex> mutexLock(pool[query].mutex); pool[query].set(std::chrono::time_point_cast<std::chrono::nanoseconds>(
pool[query].data = std::chrono::time_point_cast<std::chrono::nanoseconds>( std::chrono::system_clock::now()).time_since_epoch().count());
std::chrono::system_clock::now()).time_since_epoch().count();
} }
} // namespace vk } // namespace vk
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
#ifndef VK_QUERY_POOL_HPP_ #ifndef VK_QUERY_POOL_HPP_
#define VK_QUERY_POOL_HPP_ #define VK_QUERY_POOL_HPP_
#include "System/Synchronization.hpp"
#include "VkObject.hpp" #include "VkObject.hpp"
#include <atomic> #include <atomic>
#include <condition_variable> #include <condition_variable>
...@@ -23,8 +25,13 @@ ...@@ -23,8 +25,13 @@
namespace vk namespace vk
{ {
struct Query class Query
{ {
public:
static auto constexpr INVALID_TYPE = VK_QUERY_TYPE_MAX_ENUM;
Query();
enum State enum State
{ {
UNAVAILABLE, UNAVAILABLE,
...@@ -32,12 +39,53 @@ struct Query ...@@ -32,12 +39,53 @@ struct Query
FINISHED FINISHED
}; };
std::mutex mutex; struct Data
std::condition_variable condition; {
State state; // guarded by mutex State state; // The current query state.
int64_t data; // guarded by mutex int64_t value; // The current query value.
std::atomic<int> reference; };
VkQueryType type;
// reset() sets the state of the Query to UNAVAILABLE, sets the type to
// INVALID_TYPE and clears the query value.
// reset() must not be called while the query is in the ACTIVE state.
void reset();
// prepare() sets the Query type to ty, and sets the state to ACTIVE.
// prepare() must not be called when the query is already ACTIVE.
void prepare(VkQueryType ty);
// start() begins a query task which is closed with a call to finish().
// Query tasks can be nested.
// start() must only be called when in the ACTIVE state.
void start();
// finish() ends a query task begun with a call to start().
// Once all query tasks are complete the query will transition to the
// FINISHED state.
// finish() must only be called when in the ACTIVE state.
void finish();
// wait() blocks until the query reaches the FINISHED state.
void wait();
// getData() returns the current query state and value.
Data getData() const;
// getType() returns the type of query.
VkQueryType getType() const;
// set() replaces the current query value with val.
void set(int64_t val);
// add() adds val to the current query value.
void add(int64_t val);
private:
sw::WaitGroup wg;
sw::Event finished;
std::atomic<State> state;
std::atomic<VkQueryType> type;
std::atomic<int64_t> value;
}; };
class QueryPool : public Object<QueryPool, VkQueryPool> class QueryPool : public Object<QueryPool, VkQueryPool>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment