[SiRoB]

First sorry to netfinity, i can't resist...

Stulle, on Mar 7 2006, 10:36 PM, said:

tHeWiZaRdOfDoS, on Mar 7 2006, 08:21 PM, said:

@Netfinity: just as a proposal: replace UPLOAD_CLIENT_DATARATE by 3072 because a lot of mods modify that variable and then the feature won't work correctly

thought you don't like those "C&P mods"¿¡ why do you care, though¿

Seems, he is trying to baby sitting his community as always.

[netfinity]

Thanks everyone, I'll fix those things for the next build!

I was looking into adding simple slot limiting too to the next release, but some tests I'm running shows that the upload-bandwidth throttler doesn't request more slots when it need to, so it is easy to end up with too few slots. Well I could do it, but then the code would be more complex than I want it.

Hope to have a bugfixed build out in a few days...

/netfinity

[BlueSonicBoy]

tHeWiZaRdOfDoS, on Mar 7 2006, 03:21 PM, said:

@BlueSonicBoy: else isn't necessary but saves one comparison

I know it isn't necessary and it only saves a comparison when the first term is true, so it doesn't save much, but it costs nothing...
I was in a clock cycle saving mood...
That said this would probably save more:

Quote

bytesPerRequest = (sourceDatarate * timeToFileCompletion) >> 1;

[Thievery]

netfinity do you plan on doing an update to 1.0b to include the feedback you got?

cheers

[netfinity]

@Thievery

Yes, I do! Probably I will have the new build ready before Monday.

Regards,
netfinity

[BlueSonicBoy]

I am really tired, not the best time to look at code, but I was looking and...

Quote

// If this gap has not already been requested, we have found a valid entry
#ifndef DONT_USE_DBR
// BEGIN netfinity: DynamicBlockRequests - Reduce bytes to request
  bytesToRequest -= bytesToRequest % 10240;
  if (bytesToRequest < 10240) bytesToRequest = 10240;
  if (bytesToRequest > EMBLOCKSIZE) bytesToRequest = EMBLOCKSIZE;
  if((start + bytesToRequest) <= end && (end - start) > (bytesToRequest + 3072)) // Avoid creating small fragments
  end = start + bytesToRequest - 1;
// END netfinity: DynamicBlockRequests - Reduce bytes to request
#endif

Aren't these lines redundant, bacause you have already made that check in GetNextRequestedBlock?

[tHeWiZaRdOfDoS]

You are right... or at least one could add an "else" before the 2nd if 'cos it's impossible that both cases occur simultaneously *g*

[BlueSonicBoy]

tHeWiZaRdOfDoS, on Mar 24 2006, 05:47 AM, said:

You are right... or at least one could add an "else" before the 2nd if 'cos it's impossible that both cases occur simultaneously *g*

Yeah.. I'd already put the else there!
I was looking for the cause of something weird that my Mod did, and I noticed this. I think Netfinity has put the check there to protect against someone adding code which calls that function with an invalid value, but in the code as-is it is redundant.

[netfinity]

Yup, it is redundant. It's just to be on the safe side.

There is one important thing that isn't mentioned in the Mod code, and that is to comment out the AddDebugLogLine() after shrinking blocks in CPartFile::GetTotalGapSizeInPart. Otherwise it may cause some nasty exceptions. Alternatively you can fix the %i to do 64 bit.

Quote

            bool shrinkSucceeded = ShrinkToAvoidAlreadyRequested(tempStart, tempEnd);
            if(shrinkSucceeded) {
    //AddDebugLogLine(false, _T("Shrunk interval to prevent collision with already requested block: Old interval %i-%i. New interval: %i-%i. File %s."), start, end, tempStart, tempEnd, GetFileName());

See if I get some time to compile a new build, have been all to busy with other stuff.

/netfinity

[tHeWiZaRdOfDoS]

Ah hehe I already changed that part... don't forget to change the 3nd and 4th %i :flowers
Btw. I changed this one:

Quote

const bool shrinkSucceeded = ShrinkToAvoidAlreadyRequested(tempStart, tempEnd);   
    if(shrinkSucceeded)
    {
    const bool realSucceeded = (start != tempStart) || (end != tempEnd);
    if(realSucceeded && thePrefs.GetLogUlDlEvents())
      AddDebugLogLine(false, _T("Shrunk interval to prevent collision with already requested block: Old interval %i64-%i64. New interval: %i64-%i64. File %s."), start, end, tempStart, tempEnd, GetFileName());

    // Was this block to be returned

[SiRoB]

And what about, always start to request 10240 at first step ?
In particular case we still request a full block but the source is realy realy slot, it may takes 25minutes to download this 180kB block in the worst case.
I tested this change and it seems to work as expected.
Maybe you will get a better idea.

Quote

uint64 bytesPerRequest = EMBLOCKSIZE;
#if !defined DONT_USE_DBR
uint64 bytesLeftToDownload = GetFileSize() - GetCompletedSize();
uint32 fileDatarate = max(GetDatarate(), UPLOAD_CLIENT_DATARATE); // Always assume file is being downloaded at atleast 3 kB/s
uint32 sourceDatarate = max(sender->GetDownloadDatarate(), 10); // Always assume client is uploading at atleast 10 B/s
uint32 timeToFileCompletion = max((uint32) (bytesLeftToDownload / (uint64) fileDatarate) + 1, 10); // Always assume it will take atleast 10 seconds to complete

bytesPerRequest = (sourceDatarate * timeToFileCompletion) / 2;

if (bytesPerRequest > EMBLOCKSIZE)
  bytesPerRequest = EMBLOCKSIZE;
if (bytesPerRequest < 10240 || sender->GetDownloadDatarate() == 0)
{
  // Let an other client request this packet if we are close to completion and source is slow
  // Use the true file datarate here, otherwise we might get stuck in NNP state
  if (!requestedblocks_list.IsEmpty() && timeToFileCompletion < 30 && bytesPerRequest < 3400 && 5 * sourceDatarate < GetDatarate())
  {
  DebugLog(_T("No request block given as source is slow and file near completion!"));
  return false;
  }
  bytesPerRequest = 10240;
}
#endif
// BEGIN netfinty: Dynamic Block Requests

[BlueSonicBoy]

SiRoB, on Mar 26 2006, 02:44 PM, said:

And what about, always start to request 10240 at first step ?
In particular case we still request a full block but the source is realy realy slot, it may takes 25minutes to download this 180kB block in the worst case.
I tested this change and it seems to work as expected.
Maybe you will get a better idea.

I see your point, but I feel Netfinity has allowed for this, however my maths ability is not the greatest so bear with me...

Quote

Ok, were going to work out the minimum data remaining that will result if a block of EMBLOCKSIZE

uint64 bytesPerRequest = EMBLOCKSIZE;
#if !defined DONT_USE_DBR
uint64 bytesLeftToDownload = GetFileSize() - GetCompletedSize();

bytesLeftToDownload is the value we are trying to find

uint32 fileDatarate = max(GetDatarate(), UPLOAD_CLIENT_DATARATE); // Always assume file is being downloaded at atleast 3 kB/s

Here we would have 3Kb  as a MINIMUM so fileDatarate = 3072

uint32 sourceDatarate = max(sender->GetDownloadDatarate(), 10); // Always assume client is uploading at atleast 10 B/s

Here if GetDownloadDatarate() == 0 sourceDatarate = 10

uint32 timeToFileCompletion = max((uint32) (bytesLeftToDownload / (uint64) fileDatarate) + 1, 10); // Always assume it will take atleast 10 seconds to complete

so in this case where bytesLeftToDownload>30720 we have timeToFileCompletion =  (bytesLeftToDownload / 3072) + 1

bytesPerRequest = (sourceDatarate * timeToFileCompletion) / 2;


so transpose for timeToFileCompletion. Use bytesPerRequest = EMBLOCKSIZE in this case
  timeToFileCompletion    = (bytesPerRequest *2) / sourceDatarate
  timeToFileCompletion    = (bytesPerRequest *2) / 10 
Drop in EMBLOCKSIZE and we get
(184320*2)/10  = 36864

Drop that back in here: timeToFileCompletion =  (bytesLeftToDownload / 3072) + 1 and transpose bytesLeftToDownload = (timeToFileCompletion - 1) * 3072

(36864-1) * 3072 = 113243136  (110.5Mb)

So if my ropey maths is right , any file with less that 110.5Mb left will always result in a block of less that EMBLOCKSIZE for sender->GetDownloadDatarate() == 0
Effectively adding if(sender->GetDownloadDatarate() == 0) bytesPerRequest = 10240; would just increase overhead with little endgame benifit IMHO!

[leexgx]

i thuoght that the smaller request asking is only done at the end of the file (last 9mb) to not make more overhead

[BlueSonicBoy]

leexgx, on Mar 27 2006, 08:08 AM, said:

i thuoght that the smaller request asking is only done at the end of the file (last 9mb) to not make more overhead

AFAIK,it's progressive, it will have a more prononced effect the closer to the end of the download you get. What I wrote above shows the cutoff point for any effect.

[netfinity]

Yes, it is progressive!

The additional overhead caused by smaller blocks is insignificant, but the disk load at the sources increases significantly so there is a reason why it's not always 10kB. Also when connecting to horde clients you would want to ask as much as possible as you only get one block from them.


By the way, what are your opinion about the performance of this new DBR code compared with the old one and/or the official.

[leexgx]

i think your mod fix's something that should of been done an long time ago (other p2p appas i used do not do what emule does at the end of file)

your patch will allso if new users use it will not make them run away when files do not compleat (as alot i have talked to)

there are other things that need working on as well like auto upload on default settings (not uss) better wizard most users do not have an clue what the settings mean or what thay do if set not right (rant mode realy hehe not related to this mod)

[BlueSonicBoy]

netfinity, on Mar 27 2006, 11:11 AM, said:

By the way, what are your opinion about the performance of this new DBR code compared with the old one and/or the official. 

DBR addresses a problem that the official code does not address at all.
This version is so small and elegant, I believe it should be included in the next official version.

[SiRoB]

@BlueSonicBoy
Nice mathematical demonstration, i already know it.
But you don't understand my purpose.
I will make a proper change to correctly work arround what i noticed to improve not only the end completion.

[BlueSonicBoy]

SiRoB, on Mar 28 2006, 06:38 AM, said:

@BlueSonicBoy
Nice mathematical demonstration, i already know it.
But you don't understand my purpose.
I will make a proper change to correctly work arround what i noticed to improve not only the end completion.

That's cool, but if you are outside of the DBR code's working range (110Mb) the next block(s) requested will be EMBLOCKSIZE and there are drawbacks to starting a 10k too, (The main one I didn't even think of until Netfinity pointed it out! ).

Netfinity's more complex DBR code address the issue of end of Part I believe.
AFAIK the idea of this code is to fix the worst effected point of a DL and hopefully get the fix into the official.

As I see it, making a small request of a client when there is plenty of the part you are requesting the block from left is of little help in most cases. However if you are down to the last few blocks in a part then it's a good idea.

But I think this should be outside of the DBR code.
Something like...?

bool CPartFile::GetNextRequestedBlock(CUpDownClient* sender, on Requested_Block_Struct** newblocks, uint16* count), said:

// BEGIN netfinty: Dynamic Block Requests
uint64 bytesPerRequest = EMBLOCKSIZE;
#if !defined DONT_USE_DBR
uint64 bytesLeftToDownload = GetFileSize() - GetCompletedSize();
uint32 fileDatarate = max(GetDatarate(), UPLOAD_CLIENT_DATARATE); // Always assume file is being downloaded at atleast 3 kB/s
uint32 sourceDatarate = max(sender->GetDownloadDatarate(), 10); // Always assume client is uploading at atleast 10 B/s
uint32 timeToFileCompletion = max((uint32) (bytesLeftToDownload / (uint64) fileDatarate) + 1, 10); // Always assume it will take atleast 10 seconds to complete

//bytesPerRequest = (sourceDatarate * timeToFileCompletion) / 2;
bytesPerRequest = (sourceDatarate * timeToFileCompletion) >> 1;

  if (bytesPerRequest > EMBLOCKSIZE) bytesPerRequest = EMBLOCKSIZE;
  else
if (bytesPerRequest < 10240)
{
  // Let an other client request this packet if we are close to completion and source is slow
  // Use the true file datarate here, otherwise we might get stuck in NNP state
  if (!requestedblocks_list.IsEmpty() && timeToFileCompletion < 30 && bytesPerRequest < 3400 && 5 * sourceDatarate < GetDatarate())
  {
  DebugLog(_T("No request block given as source is slow and file near completion!"));
  return false;
  }
  bytesPerRequest = 10240;
}
#endif
// END netfinty: Dynamic Block Requests


// Main loop
uint16 newBlockCount = 0;
while(newBlockCount != *count){
  // Create a request block stucture if a chunk has been previously selected
  if(sender->m_lastPartAsked != (uint16)-1){
        //Reduce end of part block size for first block/slow source
      if(bytesPerRequest == EMBLOCKSIZE && sender->IsEmuleClient() && sender->GetDownloadDatarate()<400 && DataRemainingInPart(sender->m_lastPartAsked)<3072000)
            {
              bytesPerRequest = 30720;
              }
  Requested_Block_Struct* pBlock = new Requested_Block_Struct;
  if(GetNextEmptyBlockInPart(sender->m_lastPartAsked, pBlock, bytesPerRequest) == true){
    // Keep a track of all pending requested blocks
    requestedblocks_list.AddTail(pBlock);
    // Update list of blocks to return
    newblocks[newBlockCount++] = pBlock;
    // Skip end of loop (=> CPU load)
    continue;
  }
  else {

uint32 CPartFile::DataRemainingInPart(uint16 m_PartAsked) said:

uint32 CPartFile::DataRemainingInPart(uint16 m_PartAsked)
{
/* Calculate how much of the part is left in bytes (based on code from GetNextRequestedBlock() )*/
  // Offsets of 'this' chunk
  const uint64 uStart  = (uint64)m_PartAsked * PARTSIZE;
  const uint64 uEnd    = (GetFileSize() - (uint64)1 < (uStart + PARTSIZE - 1)) ?
        GetFileSize() - (uint64)1 : (uStart + PARTSIZE - 1);

  ASSERT( uStart <= uEnd );
  if(uStart >= uEnd) return PARTSIZE;

  uint32 remainingdata = 0;

  //gets bytes remaining
  for(POSITION pos = gaplist.GetHeadPosition(); pos != NULL; )
    {
  const Gap_Struct* cur_gap = gaplist.GetNext(pos);
  //Check if Gap is into the limit
  if(cur_gap->start < uStart)
    {
    if(cur_gap->end > uStart && cur_gap->end < uEnd) remainingdata += cur_gap->end - uStart + 1;
    else if(cur_gap->end >= uEnd) return PARTSIZE;
    } else
        if(cur_gap->start <= uEnd)
            {
    if(cur_gap->end < uEnd) remainingdata += cur_gap->end - cur_gap->start + 1;
      else      remainingdata += uEnd - cur_gap->start + 1;
    }
  }
 
return remainingdata;
}

edit:thresholds from 200 to 400 / 2048000 to 3072000

This post has been edited by BlueSonicBoy: 29 March 2006 - 03:11 AM

[SiRoB]

That what i wanted to point.
But we are still sending a full block on a first attempt.
If in that chunk we already got a good source we are blocking 180kB that may have a great chance to be canceled during a worst case of 25.6min (trickle source).

Anyway there is an other point to take into account.
We may alow full block size for high speed user let say the one (or more) above the average file speed per client. This may help an even faster end game and avoid round trip time request.