On 17 October 2014 15:49, Rostislav Khlebnikov
<rostislav.khlebni...@kcl.ac.uk
<mailto:rostislav.khlebni...@kcl.ac.uk>> 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
<rostislav.khlebni...@kcl.ac.uk
<mailto:rostislav.khlebni...@kcl.ac.uk>> 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
<rostislav.khlebni...@kcl.ac.uk
<mailto:rostislav.khlebni...@kcl.ac.uk>> 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"
<m.es...@ucl.ac.uk <mailto:m.es...@ucl.ac.uk>> 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
> mitk-users@lists.sourceforge.net
<mailto:mitk-users@lists.sourceforge.net>
> 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
mitk-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mitk-users
