On 2015-06-09 22:52, Francisco Jerez wrote:
+
+   if (blocking)
+      hev().wait();
+

hard_event::wait() may fail, so this should probably be done before the
ret_object() call to avoid leaks.

Alright... C++ exceptions are a minefield. :)

Is there any reason you didn't make
the same change in clEnqueueReadBuffer() and clEnqueueWriteBuffer()?


Must be an oversight. I think I did that, or at least I intended to do so.

Same comment as above.  Also note that this is being more strict than
the spec requires (which I believe is what Tom was referring to).  From
the CL 1.2 spec:

| If blocking_write is CL_TRUE, the OpenCL implementation copies the data
| referred to by ptr and enqueues the write operation in the
| command-queue. The memory pointed to by ptr can be reused by the
| application after the clEnqueueWriteBufferRect call returns.

The spec is giving you no guarantee that the write to the actual memory
object will be complete by the time the clEnqueueWriteBufferRect call
returns -- Only that your data will have been buffered somewhere and the
memory pointed to by the argument can be reused immediately by the
application.  The reason why I was reluctant to make this change last
time it came up was that it's likely to hurt performance unnecessarily
because the wait() call blocks until *all* previous commands in the same
queue have completed execution, even though in the most common case the
copy is performed synchronously using soft_copy_op(), so the wait() call
is redundant even for blocking copies.


OK, maybe we could drop the wait completely for all of the "write" calls.

The case with blocking reads is similar, the copy is handled
synchronously using soft_copy_op() when no user events are present in
the list of dependencies, so calling wait() on the event is unnecessary
to guarantee that the execution of the read has completed, and will
cause a pipe_context flush and wait until the most recent fence is
signalled.


I think it's reasonable to expect that the event is ready for profile queries after a blocking read has finished. That was the initial motivation for this patch. Other implementations behave like that. I didn't expect wait() to completely flush everything. Won't that cause a lot of needless flushing with event wait lists?

Ideally we would have a weaker variant of event::wait()
(e.g. wait_signalled()) that doesn't flush and just waits for the
associated action call-back to have been executed without giving any
guarantees about the corresponding GPU command.  The event interface
doesn't expose such a functionality right now, I'm attaching two
(completely untested) patches implementing it, you should be able to use
them as starting point to fix blocking transfers.


Thanks, I'll look into that later when I get some free time.

Grigori
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to