ktorrent 2.2beta1

Thank you for adding a way to do proper full preallocation of files. Fragmentation has been the only issue stopping me from using ktorrent as my primary torrent client.

However it does not always work. When downloading a multi file torrents the preallocation only takes place once, when the torrent is first opened. I a file is later selected for download then the ftruncate-method is used, which obviously still results in fragmented files.

After a very quick look in the code I am guessing that the call to bt::TruncateFile() in multifilecache.cpp should also take the parameter !Settings::fullDiskPrealloc(), like the call in cachefile.cpp.

We plan on adding this feature after the KDE 4 port.

This is not a request for a new feature that needs to be added. This is a bug in a feature that is scheduled for the 2.2 release.

It says in the list of new features "Option to fully preallocate diskspace to avoid fragmentation". If this is not fixed then I think you should add a note there that mentions that preallocation is only intended to work when opening a new torrent. The text in the settings menu should probably also be changed from "Fully preallocate diskspace" to "Fully preallocate diskspace when opening a new torrent" or something similar.

A much better solution would be to simply make the feature work as expected. Here is a small patch that does that.

Code: Select all

diff -ur ktorrent-2.2beta1.orig/libktorrent/torrent/multifilecache.cpp ktorrent-2.2beta1/libktorrent/torrent/multifilecache.cpp
--- ktorrent-2.2beta1.orig/libktorrent/torrent/multifilecache.cpp       2007-05-24 19:38:59.000000000 +0200
+++ ktorrent-2.2beta1/libktorrent/torrent/multifilecache.cpp    2007-06-11 01:10:14.000000000 +0200
@@ -35,6 +35,7 @@
 #include "cachefile.h"
 #include "dndfile.h"
 #include "preallocationthread.h"
+#include "settings.h"



@@ -575,7 +576,7 @@

                        if(! res)
                        {
-                               bt::TruncateFile(output_file,tf->getSize());
+                               bt::TruncateFile(output_file,tf->getSize(),!Settings::fullDiskPrealloc());
                        }
                }
                catch (bt::Error & e)
diff -ur ktorrent-2.2beta1.orig/libktorrent/util/fileops.cpp ktorrent-2.2beta1/libktorrent/util/fileops.cpp
--- ktorrent-2.2beta1.orig/libktorrent/util/fileops.cpp 2007-05-24 19:39:00.000000000 +0200
+++ ktorrent-2.2beta1/libktorrent/util/fileops.cpp      2007-06-11 01:09:24.000000000 +0200
@@ -390,7 +390,7 @@
                }
        }

-       void TruncateFile(const QString & path,Uint64 size)
+       void TruncateFile(const QString & path,Uint64 size,bool quick)
        {
                int fd = ::open(QFile::encodeName(path),O_RDWR | O_LARGEFILE);
                if (fd < 0)
@@ -398,7 +398,7 @@

                try
                {
-                       TruncateFile(fd,size,true);
+                       TruncateFile(fd,size,quick);
                        close(fd);
                }
                catch (...)
diff -ur ktorrent-2.2beta1.orig/libktorrent/util/fileops.h ktorrent-2.2beta1/libktorrent/util/fileops.h
--- ktorrent-2.2beta1.orig/libktorrent/util/fileops.h   2007-05-24 19:39:00.000000000 +0200
+++ ktorrent-2.2beta1/libktorrent/util/fileops.h        2007-06-11 01:08:22.000000000 +0200
@@ -120,7 +120,7 @@
         * @param quick Use the quick way (doesn't prevent fragmentationt)
         * @throw Error if the file doesn't exist, or something else goes wrong
         */
-       void TruncateFile(const QString & path,Uint64 size);
+       void TruncateFile(const QString & path,Uint64 size,bool quick);

        /**
         * Special truncate for FAT file systems.


I am running with this patch now, and so far it seems to work as expected. The patch is also available here, in case the forum messes up the formatting. http://pastebin.ca/raw/557498

I really hope that you can include this in 2.2.

/Pär

If it was this easy we would have already done this.

Ask yourself this question :

What would happen when the file is 5 GB large and you need to fully preallocate it ?

It would have been much more nice of you to simply tell me what you think the problem is. Now I have to guess...

The only problem that I am aware of is that since the preallocation takes some time, this method can make the GUI freeze a while until the allocation is finished. For files up to a few hundred megabytes this will not be very noticeable, but this off course gets more annoying when many, and/or bigger files is selected, and also on machines with slower disks.

The proper way would be to run the allocation in a background thread, and this would be a great improvement in some future version. But I think the behaviour of my patch could be acceptable for now, as it is much better than doing no preallocation at all some times. The setting for full preallocation will not be the default (correct?) so users who enable it will probably be able to figure out why the gui is slow when they are selecting new files. Multifile torrents containing several very large files (your 5G example) is also not very common at all.

A common usability principle is to always do the least surprising thing. If a user have explicitly selected the option to do full preallocation I would say it is very surprising that this is not done. Not understanding that the preallocation is not performed, and later discovering a file system with ruined performance because of fragmentation would also be very surprising.

If you really won't even consider this for 2.2 I think you should mention this in the release notes and clarify the settings menu as I mentioned in my previous post, that at least makes the behaviour documented and then hopefully users don't get surprised.

Bingo, this needs to be done in the background, blocking the GUI is not acceptable even if we document it. It would block all the other running torrents to.

We have code to do preallocation in the background, but this is only used when the torrent is started up. Using this in the middle of a running torrent is a big change, and I'm not going to do that now.

As I mentioned I think that would be better than no allocation, but it is off course ok to disagree I guess you haven't had as bad experiences with fragmented ext3 file systems as I have.

However PLEASE read again what I said about documentation. I am not saying that you should take my patch and document that. I am saying that you should change the documentation of the CURRENT behavior, that is the behavior WITHOUT my patch.

Now the web page have "Option to fully preallocate diskspace to avoid fragmentation" in the list of new features, and the settings menu says "Fully preallocate diskspace (avoids fragmentation)". This really do imply that preallocation will always happen, which not true yet. So having this text is very bad.

Don't get me wrong, it is great that you have implemented the current preallocation! But it needs to be documented correctly to clearly state that the preallocation only happens when a torrent is opened. This would use the trick of turning a bug into a feature by documenting it