Hi Rostislav,
I have not realised that this locking mechanism is only for images. I work
only with images at the moment but we would need the autosave to work for
other data as well, for other projects.
What regards the images, I do not really like the shallow clone approach. I
do not think it is good to save an image while it is being modified, even
if the size of the image does not change and it does not lead to a crash.
Luckily, the locking mechanism would prevent this. The autosave thread
would put a read lock on the image, i.e. no one could make any modification
on the image while it is being saved. So, this is out of question.
The only exception is if the autosave thread creates the read accessor with
the IgnoreLock flag. But it is a kind of undermining the whole locking
mechanism, and we should not do that. I am not sure that the size of the
image can be changed, but if yes, it can cause a crash, as you pointed it
out. Or it can cause that we save an invalid state (shallow clone).
http://docs.mitk.org/nightly-qt4/classmitk_1_1ImageAccessorBase.html
If the saving to the disk is slow and we can also clone the image in the
memory, but the read lock would be applied in this case as well, i.e. the
image cannot be modified during the cloning. All is safe.
How I see:
The locking mechanism should be extended for other kinds of data as well.
Without it, you cannot totally exclude the possibility that that e.g. a
planar polygon is modified while it is being either saved or even just
cloned.
In addition to this, there can be three possible autosave policies, maybe
depending on the data type or a property of a data node:
1. The data is locked for reading by the auto-save process while it is
being saved to disk.
2. The data is locked for reading by the auto-save process while it is
being cloned in memory, and it is saved to disk later.
3. The data is locked for reading by the auto-save process with the
IgnoreLock flag, and the thread starts listening to modification of the
data at the same time. If the data is modified during the cloning or
saving, it should be interrupted or restarted a bit later.
What do you think?
Sascha,
if you are still reading. :) Are you planning #14866 for the coming
release? As I see, the last merge was around the date when you did the
feature freeze.
Do you think it would be complicated to introduce a read/write locks for
non-imaging data?
Cheers,
Miklos
On 17 October 2014 13:50, Rostislav Khlebnikov <
[email protected]> wrote:
> Hi Miklos,
>
> yes, indeed it's a good feature that would be really nice to have.
>
> However, I feel that we cannot really go for just one of the approaches
> that you have described. I feel several auto-save policies would have to be
> implemented for the auto-save to always work correctly and quickly enough.
> There are several things that have to be considered here. First and
> foremost, auto-save should never lead to a crash. This means that a safe
> bet would be the cloning approach. I would say that it would work for my
> use case for a majority of data types invloved as the data are relatively
> small compared to the image size. However, it is quite clear that it
> wouldn't be a good idea to use this approach for image data, so we would
> have to consider a more involved approach here.
>
> How I see it, it is necessary to define the changes to the image data that
> would be considered "breaking". I think that most likely these would be
> only the changes in the image size that would lead to reallocation of
> memory and thus a potential crash in the saving thread which might try to
> read data from the free'd memory. The changes to the pixel data would be
> fine. I mean, the saved image might be corrupted in terms of the actual
> data, but it's auto-save after all and if for some reason app crashes, we
> will restore what can be restored. If the app doesn't crash - then the next
> auto-save or a proper save will overwrite the incorrect data. This leads me
> to think that the good approach here would be kind of a "shallow clone" of
> the image data for auto-saving purposes. This will include the image
> meta-data, but the pointer to the actual data will remain intact during
> auto-save. If the image is reallocated during autosave - then the old data
> will be freed as soon as autosave finishes. I don't work a lot with images
> and the implementation details regarding locks on the image data that you
> describe is not quite clear to me, but I think this should be quite doable.
>
> For other data types, a full clone is likely a better approach. Consider
> planar polygon for example. If the user removes a point during auto-save -
> an exception or a crash is very likely to happen.
>
> I would say that there should be two general policies (no auto-save or
> full clone) and a "custom" policy, such as shallow clone that is specific
> for image data type. With this, if someone needs an even more sophisticated
> approach - such as auto-saving only the portion of the image changed since
> last auto-save - then it would be possible to implement this. I would also
> make the auto-save policy as a BaseData or even DataNode property. The
> reason for this is that for some data objects, we may want to, e.g.,
> disable the auto-save completely. For example, this might be the derived
> data that can be easily re-created - like a surface that is extracted from
> a segmented image.
>
> So I think you could start by looking into this shallow-cloning of image
> data and a simle autosaver class which would from time to time make a clone
> of the data storage (with shallow clones where necessary) and save it in
> separate thread.
>
> In any case, I think we should only start working on this when 2014.10 is
> out, especially given that there's work being done on this bug:
> http://bugs.mitk.org/show_bug.cgi?id=14866.
>
> Hope this makes any sense,
> Rostislav.
>
>
>
>
> On 16/10/2014 19:03, Miklos Espak wrote:
>
> Hi Rostislav,
>
> it seems there is demand for this feature. :)
>
> It's good that now the locking mechanism issues have been sorted for this
> release because that would prevent concurrent access to the data. Now we
> have several options.
>
> The data can be modified only when someone puts a write lock on it. Until
> the lock is released, no-one can get either read or write lock. I guess, if
> the auto-saving would be scheduled for this time, that thread would simply
> block until the write lock is released. This can be good, but it assumes
> that the applications lock the images only for the actual time when they
> are modifying the data.
>
> Other option is that the auto-save thread creates its read accessors
> manually with the "IgnoreLock" option (not using the ITK access functions).
> Then the auto-save thread would not block, but it will need to listen to
> the modifications of the input data and restart the saving process when it
> happens. Or clone the before the saving. Not much better, although there
> less chance that the data is modified during cloning.
>
> I would go for the first option, because that is simpler. Then either
> people can fix there apps if the blocking occurs, or we can go for the more
> complicated and hacky way with the IgnoreLock flag and listening to
> Modified events.
>
> What you think?
>
> Cheers,
> Miklos
>
>
>
> On 15 October 2014 13:15, Khlebnikov, Rostislav <
> [email protected]> wrote:
>
>> Hi Miklos,
>>
>> This goes in line with my earlier email regarding the incremental saving
>> (meaning saving only the stuff that changed since the scene was opened). I
>> believe that implementing this is relatively important before implementing
>> auto-save as it will speed up the saving process significantly and will
>> reduce load on the hard drive. At least in my case where 80% of the project
>> file is the original image data that never changes.
>>
>> I will start working on this very soon - I wanted to wait until the new
>> release but likely I will just start working on this in the current master
>> if it builds correctly.
>>
>> I guess we could work on this in parallel. It'd be great if you could
>> figure out how to handle new changes to data nodes while they are being
>> auto-saved in a separate thread. Should a data clone be made (likely too
>> much memory consumption)? Should the writers support interruption of the
>> saving process? What do you think?
>>
>> I would then concentrate on supporting the separate "open project" and
>> "import data" actions, support for time stamp tracking to detect what
>> really changed, and re-packing only changed data on save.
>>
>> How I saw the auto-save working then was - "open project" - save the
>> location of temp folder used for scene loading as well as record that in
>> the persistent storage using QSetting-like mechanism. During work - save
>> the changes to this folder (will also speed up the normal saving process as
>> only changes since last auto save would have to be written to temp folder
>> and packing would have to be performed). On fresh start - check if the exit
>> was clean and if temp folder saved in persistent storage is available - try
>> to recover the scene from there.
>>
>> That's a big email I wrote :) Anyway - it'd be nice to hear from you as
>> well as MITK core developers with thoughts on this.
>>
>> Rostislav.
>>
>> > On 15 Oct 2014, at 12:38, "Miklos Espak" <[email protected]> wrote:
>> >
>> > Hi All,
>> >
>> > is anybody interested in an auto-save feature for MITK?
>> >
>> > I thought of saving the project at regular interval and restore it if
>> the application is restarted after a "non-clean" termination. :)
>> >
>> > Regards,
>> > Miklos
>> >
>> >
>> ------------------------------------------------------------------------------
>> > Comprehensive Server Monitoring with Site24x7.
>> > Monitor 10 servers for $9/Month.
>> > Get alerted through email, SMS, voice calls or mobile push
>> notifications.
>> > Take corrective actions from your mobile device.
>> > http://p.sf.net/sfu/Zoho
>> > _______________________________________________
>> > mitk-users mailing list
>> > [email protected]
>> > https://lists.sourceforge.net/lists/listinfo/mitk-users
>>
>
>
>
------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
_______________________________________________
mitk-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mitk-users
