<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Robert,</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">
<div><div class=""><div style="font-family:arial,sans-serif;font-size:13px"><div>I agree - from reading it seems like the major improvement is shifting away from a global, locking cache to a per-dataset cache. (ah. has been edited since you read it?)<br>
</div></div></div></div></div></div></blockquote><div><br></div><div>Yes, I updated the RFC some yesterday after the email from Jeff Lacoste. He forgot to hit reply all so it ended up in a different email thread, but if you haven't read it I would encourage you to read that as well. I probably should pull some of the information from that email into the RFC.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div><div class=""><div style="font-family:arial,sans-serif;font-size:13px">
<div></div></div></div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Which personally I think is worthwhile for any code/application which reads a variety of gdal datasources.</div>

<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Quick question - presumably for VRT datasets any source images currently share the global cache and are treated from this proposals' POV as their own "datasets"? As well as the VRT being a separate dataset? If so, seems like this could be quite a major win for users with VRTs for tiled images?</div>

<div style="font-family:arial,sans-serif;font-size:13px"><br></div></div></div></div></blockquote><div><br></div><div>The GDALRasterBlockManger if done per dataset is a member of the base GDALDataset Object, so all inherited classes will have it as well. So yes, I believe that should be the case that each seperate dataset in the VRT would have its own cache. All of this is assuming you set the global config option to use a per dataset cache. </div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div><div style="font-family:arial,sans-serif;font-size:13px">
</div><div style="font-family:arial,sans-serif;font-size:13px">While it's a lot of code change, it's mostly simple/repeated changes. </div><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px">> Solution 2 (RW Mutex in GDALRasterBlock)</div><div style="font-family:arial,sans-serif;font-size:13px">> Cons:</div><div style="font-family:arial,sans-serif;font-size:13px">

> It means that writing a driver is not as trivial as before and care must be taken in how locking is done within the driver in order to prevent deadlocks and maintain thread safety.</div><div style="font-family:arial,sans-serif;font-size:13px">

<br></div><div style="font-family:arial,sans-serif;font-size:13px">I'm wary about making drivers more complex, there's a lot of them. And many are unlikely to be tested in multi-threaded anger during initial development (and MT issues can be hard to unit test). Can you explain a bit further what would be the impact for typical driver styles? Can we make the typical case (no/locked multithreaded access) really easy? What sort of driver code would be needed for drivers that can do MT reads? MT reads and writes?</div>
</div></div></div></blockquote><div><br></div><div>I completely understand the being wary about making drivers more complex, there are a ton of them and I had to edit every single one for my implementation of Solution 2. However, I am starting to like this solution more and more. The reason that this solution must be this way, is because many drivers will call the same block from different bands. This can cause deadlocks if not careful because two threads on the same dataset could be in the same processes at different times. To fix this I am passing a mutex through the IReadBlock, IWriteBlock, and IRasterIO functions. This allows for a much more fine level of control over the process. It allows the mutex used to protect the Block Data pointer (that is passed through the IReadBlock, etc) to be protected only at the times required. For many drivers its simply blocking the entire time the reading/writing process is occuring. </div>
<div><br></div><div>Multi-threading reads should be possible in many cases as long as the driver code itself is safe for such an implementation. If a locking is used to protect the file pointer and only allow a single thread to be reading the file from disk at a time in most drivers they should become thread safe (also the protection of any global statics in the drivers). This means increased complexity in most drivers to make them thread safe, but I think it would be possible.</div>
<div><br></div><div>Multi-threading writes is a little more complex, and is not well covered in these changes. Issues can come up with reading and writing at the same time. Here is an example of a complex situation. </div>
<div><br></div><div>Thread 1: Writes to Block A located at (0,0), this block is then marked for deletion from the cache and is intended to be flushed.</div><div><br></div><div>Thread 1: Removes Block A from the Band's block array</div>
<div><br></div><div>Thread 2: Attempts to Read Block at (0,0). Not finding any blocks it creates Block B and inserts it to Band Block Array </div><div><br></div><div>Thread 2: Reads from the driver and loads data into Block B.</div>
<div><br></div><div>Thread 1: Writes data from Block A which is being flushed to the driver.</div><div><br></div><div>Thread 2: Edits Block B, but incorrectly because the changes from Block A are not included.</div><div><br>
</div><div>I think that this can be corrected in the code by always requiring that all flushing through the driver occurs prior to removing the block from the block array. Just checked my code and this is possible right now in Solution #2, so it might need to be something that I should fix. </div>
<div><br></div><div>In a more generally large issue, there are issues with even more simple things with writing. Imagine someone warping into a band at the same time a fill operation on a band is being executed, if there is no protection it would end up with a weird set of values. Personally I think this, is something we shouldn't attempt to fix currently as this should just be accepted as a bad way to write a multi-threaded application. If you can think of any other situations where writing might have issues I really would like to know.</div>
<div><br></div><div>Thanks,</div><div><br></div><div>Blake</div></div><br></div></div>