<div dir="ltr"><br><div class="gmail_extra">Even,<br><br><div class="gmail_quote">On Fri, Aug 22, 2014 at 1:33 PM, Even Rouault <span dir="ltr"><<a href="mailto:even.rouault@spatialys.com" target="_blank">even.rouault@spatialys.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div class="h5">
<br>
</div></div>Note: after re-reading, I realize that I misread your above sentence as "it<br>
will work to translate multiple datasets in a parallel way with a *per-<br>
dataset* cache").<br>
<br>
So, even if you didn't write it, I'm afraid that people will assume that<br>
calling CreateCopy() on the same source dataset handle would be thread-safe<br>
(imagine that one thread translates to format F1, while the other one to<br>
format F2), whereas in the current state of the RFC it is not. Because<br>
CreateCopy() will call GetGeoTransform(), GetProjectionRef() etc which are<br>
generally thread unsafe. For example GTiff has a lazy loading approach for<br>
those 2 methods, and it is not the only one. </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
 </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">If we claim thread-safety, we should likely offer full thread-safety (at least<br>

in reading scenarios), not partial one. Otherwise I'm afraid no one but the<br>
few people that have taken part to that discussion or read the RFC will know<br>
the limits.<br></blockquote><div><br></div><div>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).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
I'm wondering if it wouldn't be worth having GDALDatasetThreadSafe and<br>
GDALRasterBandThreadSafe classes (or whatever name is appropriate), that would<br>
follow the decorator pattern, i.e. they will own a thread unsafe "real"<br>
dataset/band and override the methods to lock them. Similarly to what I have<br>
done in OGR with ogr/ogrsf_frmts/generic/ogrmutexeddatasource.cpp and<br>
ogrmutexedlayer.cpp, needed for the FGDB driver (the one that depends on the<br>
ESRI SDK).<br>
<br>
My idea would be to have an open flag GDAL_OF_THREADSAFE for GDALOpenEx().<br>
When set, GDALOpen() would do the usual job and get a (in most cases) unsafe<br>
dataset object. Then it would query a virtual method of the dataset to return<br>
a thread-safe version of it (GetThreadSafe()).<br>
- If not defined by the driver, the base implementation of GetThreadSafe()<br>
would return the dataset wrapped in GDALDatasetThreadSafe<br>
- If the implementation of the dataset is already thread-safe, it's<br>
GetThreadSafe() would return just self<br>
- For the in-between situations, it could for example override<br>
GDALDatasetThreadSafe/GDALRasterBandThreadSafe base implementation to<br>
specialize the methods that don't need locking.<br>
<br>
This idea might not work (and I've not though how it would combine with my<br>
previous IReadBlock_thread_safe/IReadBlock approach). I have just written it<br>
as it came to my mind. The main motivation is to make it easy to have thread-<br>
safe versions by default, without the driver having to care about that, while<br>
being flexible to make drivers that need finer control to do it.<br></blockquote><div> </div><div>This approach worries me, there is already a lot of inheritance of the GDALDataset, its really hard to learn already. 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. </div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
If, through benchmark, we determine that the cost of the thread safe version<br>
is neglectable, then GDAL_OF_THREADSAFE might be useless, and GDALOpen() would<br>
always return the thread-safe version.</blockquote><div><br></div><div>I wish there were lots of benchmarks in the test suite (if there are, I don't know where they are), 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). </div>
</div><br></div><div class="gmail_extra">Thanks,</div><div class="gmail_extra"><br></div><div class="gmail_extra">Blake</div></div>