On 17 October 2014 15:49, Rostislav Khlebnikov <
[email protected]> wrote:
> Hi Miklos,
>
> I understand that saving corrupted data to disk is not the best approach.
> On the other hand, auto-save is likely a crash recovery mechanism, so from
> my point of view this would be the way to go as it doesn't eat up the
> memory or slow down the interaction. Honestly, this is quite hard to say
> which is a better approach without profiling the application on real data
> and looking on how annoying are the potential freezes during interaction.
> But as you say, it would be possible to implement different approaches to
> this problem so every MITK user could choose one which they like the best.
>
> I'm just afraid that if auto-saving hinders interaction, the end users
> would either complain about it a lot or, given an option, would completely
> turn off this feature and I feel this would be very sad - it's better to
> have some corrupted save than none at all. I'm thinking here about MS-word
> autosave and that oftentimes the restored file is not exactly perfect, but
> at least some of my work is saved. On the other hand, I would be very very
> annoyed if MSWord would hang even for couple of seconds during auto-save.
> But again, this is a personal preference and we could consult the end-users
> regarding that (or, perhaps, force something on them and gather feedback as
> it is usually more effective :)).
>
No, the application should not hang during auto-save. It should happen
invisibly in the background. I do not know about MS Word now, but I
remember that about 10 years ago OpenOffice stopped for the time of
autosave, and with big documents and slow computers it could take a few
seconds. It was pretty annoying.
> I would say that once we have these different approaches to saving, it
> would be nice for people to test it on their data in a real setting.
> Because it will depend on the data and the workflow. For me, the images are
> relatively big (~600^3), but they are never modified, but I work a lot with
> large surface-like data and many small data objects, such as planar
> figures. For you - it's mostly image data. Perhaps, the behavior should be
> completely different for people working with DTI data, etc.
>
> My two cents regarding the locking mechanisms for other data types. I
> think it would be really great, but I would also say - don't do that. Even
> for me with only 5-6 different BaseData subclasses, supporting this would
> be a big pain - adding code for obtaining some kind of lock before
> accessing the data in all the places seems like a lot of work and
> error-prone work at that. And if each data access is automatically
> surrounded by, say, mutex access - my gut feeling is that this would be
> detrimental to the performance of the whole application.
>
The way how it is implemented for images is completely transparent, you do
not need to add any lines of code. Maybe there is a solution that does not
need modification of client code.
Performance can be an issue, but I do not think it would be significant. Of
course, we cannot tell it until we test it.
My point is that you cannot exclude the possibility of an eventual crash if
you do not lock, you can only reduce the chance of a crash. The cloning is
faster than saving to disk, but this just means that there is less chance
that is data is modified during that. It can potentially happen that a
point is removed from a pointset while it is being cloned by the auto save
process.
So, it seems that we agree in what we want to achieve, just you are more
concerned about the performance and I am more concerned about the safety.
Hopefully, we can find a solution that is fast, does not block the GUI and
is safe at the same time. The only way to check this is if we try a couple
of options and test them.
The next MITK release has been feature freezed a week ago, so in principle
we can start the work on the current master.
all the best,
Miklos
>
> All best,
> Rostislav.
>
>
>
> On 17/10/2014 15:21, Miklos Espak wrote:
>
> 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
