Download tab (perhaps others)

mvchante · #1 (**permalink**) August 26th, 2008

Soon after my initial install of phex 3.2.6.106 , I notice poor graphics performance while files were downloading. It makes the application very frustrating to use. This is most notable on the download tab.

Upon profiling during a `lag spike`, phex.download.DownloadScopeList.add occupied 15% of the processing time. An inner class of the above, DownloadScopeComparator, occupied a further 5%. (java.nio.chaneels.FileChannel.write also occupied 5%, but I doubt it is of significance.)

phex.download.DownloadScopeList.add appears to be an O(n^2) algorithm, and n seems to get very large. Has any work been made to make it more efficient?

Is there any background information available on this topic? Any advise as to other possible causes? Can you confirm that others are experiencing the same issue?

History with product: This is my first time using phex, and I haven't used any Gnutella client in a long while.

System: Quad Core Intel x64 running Vista x64. Java build 1.6.0_03-b05

Cheers,
M. V`Chante

GregorK · #2 (**permalink**) August 29th, 2008

I think the problem was introduced by this change:
SourceForge.net Repository - [phex] Diff of /phex/trunk/src/main/java/phex/common/bandwidth/BandwidthController.java
Removing the "synchronized" keywords might fix this. Can you verify it?

mvchante · #3 (**permalink**) August 29th, 2008

Removing the synchronized added in the diff you mentioned does indeed eliminate the GUI lag; however, I presume those synchronized's are needed?

On a broader look: When the GUI renders, it gets the most current values from BandwidthController by-way-of SWDownloadFile . This causes the EventDispatchThread to block until it can retrieve this value from the data transfer thread.

As a test, I implemented a cache (see below). This showed a measurable improvement in GUI response.

I believe the ultimate solution would be to have SWDownloadFile listen to BandwidthController to receive updated values when available /instead of/ having SWDownloadFile query BandwidthController for each request and thus blocking.

Comments?

Code:



    public static class EdtCache
    {
        private final SWDownloadFile download   ;
        private final long           updateRate ;

        private long lastUpdate             ;
//        private int  shortTermTransferRate  ;
        private int  longTermTransferRate   ;
        private long transferSpeed          ;
        private long downloadThrottlingRate ;

        public EdtCache(SWDownloadFile download, long updateRate)
        {
            this.download = download;
            this.updateRate = updateRate;
            update();
        }

        private void update()
        {
try {
//            shortTermTransferRate   = download.getShortTermTransferRate  ();
            longTermTransferRate    = download.getLongTermTransferRate   ();
            transferSpeed           = download.getTransferSpeed          ();
            downloadThrottlingRate  = download.getDownloadThrottlingRate ();
            lastUpdate              = System.currentTimeMillis();
} catch (RuntimeException re) {
    re.printStackTrace();
    throw re;
}
        }

        private boolean needsUpdate() { return System.currentTimeMillis() - lastUpdate > updateRate; }

//        public int getShortTermTransferRate()
//        {
//            if (needsUpdate()) update();
//            return shortTermTransferRate;
//        }

        public int getLongTermTransferRate()
        {
            if (needsUpdate()) update();
            return longTermTransferRate;
        }

        public long getTransferSpeed()
        {
            if (needsUpdate()) update();
            return transferSpeed;
        }

        public long getDownloadThrottlingRate()
        {
            if (needsUpdate()) update();
            return downloadThrottlingRate;
        }

    }

Cheers,
M. V`Chante

GregorK · #4 (**permalink**) August 30th, 2008

Actually the synchronized use in BandwidthController is crap. Since the it would only ensure locking on the reference to TransferAverage, which is not critical. But what is more critical is the internal data of TransferAverage. The access to values[] is TransferAverage is the more "critical" section. But since all this bandwidth use is a more or less approximately calculation anyway, the differences through missing synchronization is likely irrelevant.

A event concept is a nice idea. I just fear it could generate a huge load of events. Basically each read/write operation would cause a new event to be fired. Most of them likely being ignored.

The UI is updating every 2 seconds currently, so this should ideally already provide limited access to the underlying data. I could be wrong with that though.. I just assume it.

I added some modifications to SVN, would be nice if you could test how those behave for you.

Gregor