<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
I want to start off by saying a big thanks to Blake for taking his
time to tackle what can only be a very difficult problem.<br>
<br>
From what I can observe, the current discussion seems to be around
the boundary of who should be responsible for ensuring thread safety
around the block cache. The core of GDAL versus the individual
drivers. While I see why such a conversation is important, as far as
I am concerned, the most important part should be how it affects
users of GDAL at the interface level. That is, if an application
that is threaded is trying to use GDAL, how does it ensure
thread-safety? What you have to keep in mind is that having some
parts of the library not thread-safe basically just pushes the
mutexing/locking to the calling applications.<br>
<br>
Also, while it is important to document thread-safety limitations,
might I suggest adding thread-safe related capabilities
(TestCapability), especially if all drivers do not end-up having the
same thread-safety constraints.<br>
<br>
I personally do not see a GDALDatasetThreadSafe wrapper as adding
much complexity. For instance, if you were to add a capability that
indicates if a driver is inherently thread-safe, you could add a new
open method to open a dataset in a thread-safe way with something
like the following pseudocode:<br>
<br>
<tt>DataSet OpenThreadSafe(GDALOpenInfo openInfo)</tt><tt><br>
</tt><tt>{</tt><tt><br>
</tt><tt> DataSet dataSet = Open(</tt><tt>openInfo</tt><tt>);</tt><tt><br>
</tt><tt><br>
</tt><tt> if (!dataSet.TestCapability(THREAD_SAFE))</tt><tt><br>
</tt><tt> {</tt><tt><br>
</tt><tt> dataSet = wrapWithThreadSafeWrapper(dataSet);</tt><tt><br>
</tt><tt> }</tt><tt><br>
</tt><tt><br>
</tt><tt> return dataSet;</tt><tt><br>
</tt><tt>}</tt><br>
<br>
Kind Regards,<br>
André<br>
<br>
<div class="moz-cite-prefix">On 2014-08-22 17:12, Even Rouault
wrote:<br>
</div>
<blockquote cite="mid:201408222212.57922.even.rouault@spatialys.com"
type="cite">
<pre wrap="">Le vendredi 22 août 2014 21:50:56, Blake Thompson a écrit :
</pre>
<blockquote type="cite">
<pre wrap="">Even,
On Fri, Aug 22, 2014 at 1:33 PM, Even Rouault <a class="moz-txt-link-rfc2396E" href="mailto:even.rouault@spatialys.com"><even.rouault@spatialys.com></a>
wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Note: after re-reading, I realize that I misread your above sentence as
"it will work to translate multiple datasets in a parallel way with a
*per- dataset* cache").
So, even if you didn't write it, I'm afraid that people will assume that
calling CreateCopy() on the same source dataset handle would be
thread-safe (imagine that one thread translates to format F1, while the
other one to format F2), whereas in the current state of the RFC it is
not. Because CreateCopy() will call GetGeoTransform(),
GetProjectionRef() etc which are generally thread unsafe. For example
GTiff has a lazy loading approach for those 2 methods, and it is not the
only one.
</pre>
</blockquote>
<pre wrap="">
If we claim thread-safety, we should likely offer full thread-safety (at
</pre>
<blockquote type="cite">
<pre wrap="">least
in reading scenarios), not partial one. Otherwise I'm afraid no one but
the few people that have taken part to that discussion or read the RFC
will know
the limits.
</pre>
</blockquote>
<pre wrap="">
Perhaps the goal of the RFC should be to allow CreateCopy to be threadsafe?
I have no tried yet to venture into other parts of thread safety on the
dataset (already bit off a very large chunk).
</pre>
<blockquote type="cite">
<pre wrap="">I'm wondering if it wouldn't be worth having GDALDatasetThreadSafe and
GDALRasterBandThreadSafe classes (or whatever name is appropriate), that
would
follow the decorator pattern, i.e. they will own a thread unsafe "real"
dataset/band and override the methods to lock them. Similarly to what I
have
done in OGR with ogr/ogrsf_frmts/generic/ogrmutexeddatasource.cpp and
ogrmutexedlayer.cpp, needed for the FGDB driver (the one that depends on
the
ESRI SDK).
My idea would be to have an open flag GDAL_OF_THREADSAFE for
GDALOpenEx(). When set, GDALOpen() would do the usual job and get a (in
most cases) unsafe
dataset object. Then it would query a virtual method of the dataset to
return
a thread-safe version of it (GetThreadSafe()).
- If not defined by the driver, the base implementation of
GetThreadSafe() would return the dataset wrapped in
GDALDatasetThreadSafe
- If the implementation of the dataset is already thread-safe, it's
GetThreadSafe() would return just self
- For the in-between situations, it could for example override
GDALDatasetThreadSafe/GDALRasterBandThreadSafe base implementation to
specialize the methods that don't need locking.
This idea might not work (and I've not though how it would combine with
my previous IReadBlock_thread_safe/IReadBlock approach). I have just
written it
as it came to my mind. The main motivation is to make it easy to have
thread-
safe versions by default, without the driver having to care about that,
while
being flexible to make drivers that need finer control to do it.
</pre>
</blockquote>
<pre wrap="">
This approach worries me, there is already a lot of inheritance of the
GDALDataset, its really hard to learn already.
</pre>
</blockquote>
<pre wrap="">
Well, the learning phase would be only be people who want to tweak thread
safety of their drivers, i.e. not a lot of people I bet. The user wouldn't
have to pay a lot (just specifying GDAL_OF_THREADSAFE, or nothing at all if
the price of thread safety is not too big)
</pre>
<blockquote type="cite">
<pre wrap="">Adding another layer would
make it even harder to maintain in my opinion. It also provides one more
config option in an already overwhelming number of config options. I think
we might be better off if we made a long term plan on how we wanted to make
GDAL thread safe. Perhaps make a list of parts needing thread safety and
progress towards it over a period of time. As part of it maintain a webpage
that describes what is thread safe currently, so people are not confused on
what is and is not thread safe.
</pre>
</blockquote>
<pre wrap="">
People are already confused... and have a hard-time really understanding the
implication of the few lines at
<a class="moz-txt-link-freetext" href="http://trac.osgeo.org/gdal/wiki/FAQMiscellaneous#IstheGDALlibrarythread-safe">http://trac.osgeo.org/gdal/wiki/FAQMiscellaneous#IstheGDALlibrarythread-safe</a>
I'm afraid more documentation of a more complex situation will not make it
better. It is not reasonable to have to document that "GetProjectionRef() is
thread-safe in drivers X and Y, whereas GetGeoTransform() is in Y and Z".
Because that's the situation we will have for sure if we don't adopt a global
default mechanism (unless someone spends weeks in adding locks in every
virtual method of every driver).
The external locking I suggested is not completely a new approach. I could
mention Collections.synchronizedList() used in Java to make their List
implementations thread safe... Sorry for mentionning Java ;-)
The ideal would be to have a solution were by default we have a guarantee
thread-safety and people (GDAL driver developers) who want to tune it can do
it if they are brave enough.
</pre>
<blockquote type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">If, through benchmark, we determine that the cost of the thread safe
version
is neglectable, then GDAL_OF_THREADSAFE might be useless, and GDALOpen()
would
always return the thread-safe version.
</pre>
</blockquote>
<pre wrap="">
I wish there were lots of benchmarks in the test suite (if there are, I
don't know where they are),
</pre>
</blockquote>
<pre wrap="">
They don't yet exist. Most code is functionnal code in Python, so not
appropriate for performance testing, especially regarding threading.
The autotest/cpp should likely be enhanced to have such tests (the only
performance benchmark I can think of is testperfcopywords that was created to
justify the templated approach of GDALCopyWords)
To be mentionned, there's also apps/multireadtest.cpp (not compiled by
default). It just ensures that opening the same file in N dataset objects and
reading them in N threads works.
</pre>
<blockquote type="cite">
<pre wrap="">so that we can even roughly measure changes to
the internals of the library. (I realize this is something that could be
difficult to do well).
Thanks,
Blake
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
<div class="moz-signature">-- <br>
<div style="width:600px;"> <b>André Vautour</b>, B.Sc.<br>
Application Developer<br>
<a href="http://www.caris.com"><img
src="cid:part1.07080001.05070804@caris.com" alt="CARIS"
height="45" width="106" border="0"></a>
<!--<p style="padding-top:10px;"><strong style="color:#004680;">CARIS</strong>-->
<br>
115 Waggoners Lane<br>
Fredericton, New Brunswick<br>
Canada E3B 2L4<br>
Tel: +1.506.458.8533 Fax: +1.506.459.3849<br>
<a href="http://www.caris.com">www.caris.com</a>
<p style="margin-top:26px;"> <strong>Connect with CARIS</strong><br>
<a href="http://www.twitter.com/CARIS_GIS"><img
src="cid:part4.05010108.06050809@caris.com" alt="Twitter"
height="32" width="32" border="0"></a> <a
href="http://www.linkedin.com/groups?mostPopular=&gid=3217878"><img
src="cid:part6.05020404.02050702@caris.com" alt="LinkedIn"
height="32" width="32" border="0"></a> <a
href="http://www.facebook.com/pages/CARIS-The-Marine-GIS-Experts/123907500987669?v=app_4949752878"><img
src="cid:part8.09040605.07060702@caris.com" alt="FaceBook"
height="32" width="32" border="0"></a> <a
href="http://www.youtube.com/user/CARISGIS"><img
src="cid:part10.05000506.03070700@caris.com" alt="YouTube"
height="32" width="32" border="0"></a> </p>
<p style="margin-top:26px; line-height:1.5em;">Download your
free copy of CARIS Easy View today!<br>
<a href="http://www.caris.com/easyview">www.caris.com/easyview</a></p>
<p>_________________________________________________________________________<br>
This email and any files transmitted with it are confidential
and intended only for the addressee(s). If you are not the
intended recipient(s) please notify us by email reply. You
should not use, disclose, distribute or copy this
communication if received in error.</p>
<p> Any views or opinions expressed in this email are solely
those of the author and do not necessarily represent those of
the company. No binding contract will result from this email
until such time as a written document is signed on behalf of
the company.</p>
</div>
</div>
</body>
</html>