On 17 October 2014 15:49, Rostislav Khlebnikov
<[email protected]
<mailto:[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]
<mailto:[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]
<mailto:[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] <mailto:[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]
<mailto:[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] <mailto:[email protected]>
https://lists.sourceforge.net/lists/listinfo/mitk-users
