[Enig123]

As I remember, this assert was used to debug the new UploadingToClient_Struct used in uploading queue. Since there's no problem there anymore, probably it can be just simply commented out.

[Ergol]

Some Support, on 25 April 2015 - 06:11 PM, said:

I have updated the devbuild with a new version which changes the upload slot speeds, ranging from 3KBs (the old value) for 3 open slots to 50KB/s per slot for over 40 open slots. But I haven't really had the chance to test it yet while uploading, so any feedback would be nice, esp. with fast lines - also trying unlimited, limited and USS as max speed.

It is not a trivial in general to test in the wild on 100 Mbit/s for like 40+ downloaders condition since hardly seen that many at the same time and 9900 KB/s never reached yet so no "clients waiting in queue" condition met. Some results as per below could be interesting however (dev 16; Connection-Capacities-Upload->9900 KB/s).

Check 1.

Upload limit is 9900 KB/s (no USS) and 4 downloaders - A (low id); B (low id); C; D (low id) ->
1-1. A ~ 40 KB/s; B ~ 2,29 MB/s; C ~ 317  KB/s; D ~ 20 KB/s -> then upload limit removed ->
1-2. A ~ 19 KB/s; B ~ 141  KB/s; C ~ 141  KB/s; D ~ 9  KB/s -> then upload limit set to 9900 KB/s ->
1-3. A ~ 48 KB/s; B ~ 2,39 MB/s; C ~ 1,08 MB/s; D ~ 20 KB/s

Repeated after some time with upload limit remained 9900 KB/s - C left already; E (low id) entered ->
1-4. A ~ 48 KB/s; B ~ 2,43 MB/s; E ~ 13 KB/s; D ~ 21 KB/s -> then upload limit removed ->
1-5. A ~ 20 KB/s; B ~ 285  KB/s; E ~ 8  KB/s; D ~ 8  KB/s -> then upload limit set to 9900 KB/s ->
1-6. A ~ 47 KB/s; B ~ 2,56 MB/s; E ~ 21 KB/s; D ~ 14 KB/s

The above results were quite confusing due to huge upload drop for removed limit (unlimited upload), but above was with UploadCapacity=25 leftover from previous eMule versions as discovered later comparing preferences.ini with fresh from 0.50b BETA1 install. Removed UploadCapacity=25 so only UploadCapacityNew=9900 remained and performed the further checks (Sidenote: OverlappedSockets=1 dev 16 crashed with removed UploadCapacity=25 leftover anyway).

Check 2.

Upload limit is 9900 KB/s (no USS) and 2 downloaders - A (low id); B (low id) ->
2-1. A ~ 50 KB/s; B ~ 5,7 MB/s -> then upload limit removed ->
2-2. A ~ 50 KB/s; B ~ 5,7 MB/s -> then enabled USS (lowest allowed upload speed = 1) ->
2-3. A ~ 50 KB/s; B ~ 5   MB/s -> then changed USS lowest allowed upload speed to 9900 ->
2-4. A ~ 50 KB/s; B ~ 5,7 MB/s -> then upload limit set to 1024 KB/s; disabled USS ->
2-5. A ~ 50 KB/s; B ~ 970 KB/s

Check 3.

Upload limit is 9900 KB/s (no USS) and 3 downloaders - A; B (low id); C ->
3-1.  A ~ 60 KB/s; B ~ 60  KB/s; C ~ 3,6 MB/s               -> then D (low id) entered ->
3-2.  A ~ 60 KB/s; B ~ 60  KB/s; C ~ 2,6 MB/s; D ~ 4,0 MB/s -> then upload limit removed ->
3-3.  A ~ 60 KB/s; B ~ 60  KB/s; C ~ 2,7 MB/s; D ~ 3,5 MB/s -> then enabled USS (lowest allowed upload speed = 1) ->
3-4.  A ~ 60 KB/s; B ~ 60  KB/s; C ~ 2,4 MB/s; D ~ 2,5 MB/s -> then changed USS lowest allowed upload speed to 9900 ->
3-5.  A ~ 60 KB/s; B ~ 60  KB/s; C ~ 2,6 MB/s; D ~ 3,5 MB/s -> then upload limit set to 9900 KB/s ->
3-6.  A ~ 60 KB/s; B ~ 60  KB/s; C ~ 2,6 MB/s; D ~ 4,0 MB/s -> then upload limit set to 1024 KB/s; disabled USS ->
3-7.  A ~ 15 KB/s; B ~ 30  KB/s; C ~ 660 KB/s; D ~ 320 KB/s -> then upload limit set to 9900 KB/s ->
3-8.  A ~ 30 KB/s; B ~ 60  KB/s; C ~ 2,6 MB/s; D ~ 4,0 MB/s -> then D (low id) left ->
3-9.  A ~ 30 KB/s; B ~ 60  KB/s; C ~ 3,6 MB/s               -> then B (low id) left; E (low id) entered ->
3-10. A ~ 30 KB/s; E ~ 430 KB/s; C ~ 3,6 MB/s               -> then F (low id) entered ->
3-11. A ~ 30 KB/s; E ~ 430 KB/s; C ~ 2,6 MB/s; F ~ 4,0 MB/s

Never performed such tests prior dev 16 so no info for slots vs speed to compare. Result 3-11 looks promising as higher speed downloaders A, B (C seems capped already as 3-8 vs 3-9; probably D and F were capped aswell) or more opened slots could lead to better upload capacity accommodation.

Hope it helps.

This post has been edited by Ergol: 28 April 2015 - 06:39 PM

[Some Support]

Ergol, on 28 April 2015 - 05:43 PM, said:

It is not a trivial in general to test in the wild on 100 Mbit/s for like 40+ downloaders condition since hardly seen that many at the same time and 9900 KB/s never reached yet so no "clients waiting in queue" condition met. Some results as per below could be interesting however (dev 16; Connection-Capacities-Upload->9900 KB/s).

Thanks for your work. The general upload speed and capacity should not have changed in build 16, but only the average slot speed. But of course that comes only into play when all slots are taken, otherwise the additional bandwidth is distributed to the active slots. 3-4 open slots isn't quite enough for such a test, there should be a waiting queue. But I understand that depending on which files and how many files you share this is hard to test.

[Riso64Bit]

Ergol:
try to put in your incomming folder some linux-s distro

[Ergol]

Riso64Bit, on 29 April 2015 - 10:02 PM, said:

Ergol:
try to put in your incomming folder some linux-s distro

Last time I checked they were officially distributed via torrents and not much availability as per Kad search. Noted that some up-to-date at Content-DB, but activity is quite low.

[fox88]

Several new hints from another code analyzer.

1. File DownloadClient.cpp
Method void CUpDownClient::DrawStatusBar(CDC* dc, LPCRECT rect, bool onlygreyrect, bool bFlat) const

ASSERT(reqfile);
s_StatusBar.SetFileSize(reqfile->GetFileSize());
s_StatusBar.SetHeight(rect->bottom - rect->top);
s_StatusBar.SetWidth(rect->right - rect->left);
s_StatusBar.Fill(crNeither);

if (!onlygreyrect && reqfile && m_abyPartStatus)
{

It would be safer and more logical to check reqfile for null instead or immediately after the assert to be sure about reqfile->GetFileSize().


Method void CUpDownClient::SendBlockRequests()

if (thePrefs.GetDebugClientTCPLevel() > 0)
	DebugSend("OP__RequestParts", this, reqfile!=NULL ? reqfile->GetFileHash() : NULL);

In this case condition reqfile!=NULL is superfluos, because it was already verified at the very beginning of the method.

2. File EmuleDlg.cpp
Method LRESULT CemuleDlg::OnWebGUIInteraction(WPARAM wParam, LPARAM lParam)

if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, &hToken))
	throw;

Throwing exception in that form should be used in catch() blocks, because it re-throws an existing exception.
Therefore at least something should be thrown; preferably of the class std::exception. In the very least, throw 0; might be used.

3. File IESecurity.cpp
Method STDMETHODIMP CMuleBrowserControlSite::XInternetSecurityManager::MapUrlToZone

TRACE(_T("%hs: URL=%ls, Zone=%d, Flags=0x%x\n"), "MapUrlToZone", pwszUrl, *pdwZone, dwFlags);
if (pdwZone != NULL) {

It would be safer to move TRACE() down after if() line; or change the line to:

TRACE(_T("%hs: URL=%ls, Zone=%d, Flags=0x%x\n"), "MapUrlToZone", pwszUrl, pdwZone ? *pdwZone : 0, dwFlags);

4. File IrcMain.cpp
Method void CIrcMain::ParseMessage(CString sRawMessage)

sSound.Format(_T("%sSounds\\IRC\\%s"), thePrefs.GetMuleDirectory(EMULE_EXECUTEABLEDIR), sSound);

Formatting a string into itself is at least questionable, simple concatenation would be safer:

sSound = thePrefs.GetMuleDirectory(EMULE_EXECUTEABLEDIR)+_T("Sounds\\IRC\\")+sSound;

This post has been edited by fox88: 27 June 2015 - 03:59 PM

[fox88]

eMule got assert in Entry.cpp at line 868, and stayed for a while with that message on screen.
Then I attached debugger to look at it, then tried to continue, but eMule crashed in void CListenSocket::OnAccept(int nErrorCode) while executing code

CUpDownClient* pClient = theApp.clientlist->FindClientByIP(SockAddr.sin_addr.S_un.S_addr);
AddDebugLogLine(false, _T("Rejecting connection attempt of banned client %s %s"), ipstr(SockAddr.sin_addr.S_un.S_addr), pClient->DbgGetClientInfo());

Obviously, the client already disconnected and pClient is null.
Checking for null before callig AddDebugLogLine() would be appropriate here.

[fox88]

File WebSocket.cpp,
method void CWebSocket::OnRequestReceived(char* pHeader, DWORD dwHeaderLen, char* pData, DWORD dwDataLen, in_addr inad)

bool filereq=false;
...
if (sURL.GetLength()>4 && // min length (for valid extentions)
	(sURL.Right(4).MakeLower()==".gif" || sURL.Right(4).MakeLower()==".jpg" || sURL.Right(4).MakeLower()==".png" ||
	sURL.Right(4).MakeLower()==".ico" ||sURL.Right(4).MakeLower()==".css" ||sURL.Right(3).MakeLower()==".js" ||
	sURL.Right(4).MakeLower()==".bmp" || sURL.Right(5).MakeLower()==".jpeg"
	)
	&& sURL.Find("..")==-1	// dont allow leaving the emule-webserver-folder for accessing files
	)
		filereq=true;

There is a bug: check for length above 4 yet trying to look for 3 character ".js". Besides, that string is also a valid name in Windows.

It could be rewritten as:

bool filereq = sURL.GetLength()>=3 && sURL.Find("..")==-1; // don't allow leaving the emule-webserver-folder for accessing files
if (filereq) {
	CStringA ext(sURL.Right(5).MakeLower());
	int i = ext.ReverseFind('.');
	ext.Delete(0, i);
	filereq = i>=0 && ext.GetLength()>2 && (ext==".gif" || ext==".jpg" || ext==".png" || ext==".ico" || ext==".css" || ext==".bmp"
			|| ext==".js" || ext==".jpeg");
}

Obviously, the previous definition of filereq should be removed.

This post has been edited by fox88: 27 June 2015 - 03:09 PM

[fox88]

File Emule.cpp, function int CrtDebugReportCB(int reportType, char* message, int* returnValue)

_ftprintf(fp, _T("%s  %i  %hs"), szTime, reportType, message);

Since h is an unknown modifier for %s, and szTime is wide string, the line should be:

_ftprintf(fp, _T("%ls  %i  %hs"), szTime, reportType, message);

Edit. I found a few %hs formatting codes around.
According to www.securecoding.cert.org, it should not be used.
Edit 2. %hs returned back into MS online documentation as supported in VS2013.

This post has been edited by fox88: 27 June 2015 - 03:11 PM

[fox88]

Odd line in PartFile.cpp, method void CPartFile::Init()

(void)m_strComment;

Most certainly it was intended to be

m_strComment.Empty();

Edit. There are a few such casts in the following block. Compiler omits these code lines even in debug build.
My guess it was prepared for assert checks, but remained unused.

Spoiler

#ifdef _DEBUG
void CPartFile::AssertValid() const
{
	CKnownFile::AssertValid();

	(void)m_LastSearchTime;
	(void)m_LastSearchTimeKad;
	(void)m_TotalSearchesKad;
	srclist.AssertValid();
	A4AFsrclist.AssertValid();
	(void)lastseencomplete;
	m_hpartfile.AssertValid();
	m_FileCompleteMutex.AssertValid();
	(void)src_stats;
	(void)net_stats;
	CHECK_BOOL(m_bPreviewing);
	CHECK_BOOL(m_bRecoveringArchive);
	CHECK_BOOL(m_bLocalSrcReqQueued);
	CHECK_BOOL(srcarevisible);
	CHECK_BOOL(m_bMD4HashsetNeeded);
	CHECK_BOOL(m_bAICHPartHashsetNeeded);
	(void)m_iLastPausePurge;
	(void)count;
	(void)m_anStates;
	ASSERT( completedsize <= m_nFileSize );
	(void)m_uCorruptionLoss;
	(void)m_uCompressionGain;
	(void)m_uPartsSavedDueICH; 
	(void)datarate;
	(void)m_fullname;
	(void)m_partmetfilename;
	(void)m_uTransferred;
	CHECK_BOOL(paused);
	CHECK_BOOL(stopped);
	CHECK_BOOL(insufficient);
	CHECK_BOOL(m_bCompletionerror);
	ASSERT( m_iDownPriority == PR_LOW || m_iDownPriority == PR_NORMAL || m_iDownPriority == PR_HIGH );
	CHECK_BOOL(m_bAutoDownPriority);
	ASSERT( status == PS_READY || status == PS_EMPTY || status == PS_WAITINGFORHASH || status == PS_ERROR || status == PS_COMPLETING || status == PS_COMPLETE );
	CHECK_BOOL(newdate);
	(void)lastpurgetime;
	(void)m_LastNoNeededCheck;
	gaplist.AssertValid();
	requestedblocks_list.AssertValid();
	m_SrcpartFrequency.AssertValid();
	ASSERT( percentcompleted >= 0.0F && percentcompleted <= 100.0F );
	corrupted_list.AssertValid();
	(void)availablePartsCount;
	(void)m_ClientSrcAnswered;
	(void)s_LoadBar;
	(void)s_ChunkBar;
	(void)m_lastRefreshedDLDisplay;
	m_downloadingSourceList.AssertValid();
	m_BufferedData_list.AssertValid();
	(void)m_nTotalBufferData;
	(void)m_nLastBufferFlushTime;
	(void)m_category;
	(void)m_dwFileAttributes;
}

void CPartFile::Dump(CDumpContext& dc) const
{
	CKnownFile::Dump(dc);
}
#endif

This post has been edited by fox88: 31 May 2015 - 12:27 PM

[fox88]

When calling CException::GetErrorMessage(),
Windows does not care if returned message is null terminated.

The simplest way to fix might be to add function into OtherFunctions.cpp:

BOOL GetExceptionMessage(const CException& ex, LPTSTR lpszError, UINT nMaxError)
{
	BOOL ret = ex.GetErrorMessage(lpszError, nMaxError);
	if (lpszError)
		lpszError[ret && nMaxError ? nMaxError-1 : 0] = 0;
	return ret;
}

MS recommends to use constant references in exception handlind, like
catch(const Exeption& ex) {...}. That was the reason for the first parameter declaration.

PS. At first I thought there are lots of files with that problem. Luckily, there were calls to the function with the same name as the method in question. Still about two dozen, usually with several entries.

This post has been edited by fox88: 10 May 2015 - 06:41 PM

[fox88]

fox88, on 11 October 2014 - 05:40 PM, said:

Third. The reason for rehashing was a side-effect of time zone M$ patch. Since the system time information changed, a few GB of files suddenly got "changed". Does eMule compare local times instead of internal UTC timestamps?

It was interesting to learn that eMule has special setting for that:

m_bAdjustNTFSDaylightFileTime=ini.GetBool(L"AdjustNTFSDaylightFileTime", true);

So by default eMule adjusts file time, which initially was time zone independent?
I see no reason for that static adjustment.
Obviously every DST on/off event or time zone information change will force rehashing of all your files.
If we ever need to show file timestamp, the adjustment should be done on the fly, because time shift might happen while eMule runs, and that would invalidae all previous calculations.

Edit. On second thought, I could guess why that happened.
File Times
FAT uses local time, and for that reason eMule needed to adjust time in attempt to avoid hashing.
The default global AdjustNTFSDaylightFileTime=true setting should bs fine for FAT volumes, but might be incorrect for NTFS volumes.
Also I read comments in code and found Time stamp changes with daylight savings. Time in known.met needs no adjustment as I see it.

This post has been edited by fox88: 31 May 2015 - 12:52 PM

[fox88]

Some Support, on 22 March 2014 - 01:35 AM, said:

Of course thats an obvious typo in the code, it was meant to be "CAICHHash(fileInput);". The current code really does nothing (at least nothing it is supposed to do) and is therefore also a bug - not one which should normally be triggered however.

Remember that old message?
Now there is a good reason to return to the subject of calling constructors instead of methods (bug found using code analyzer).
One of several cases in file ClientVersionInfo.h:

CClientVersionInfo(uint32 dwTagVersionInfo, UINT nClientMajor)
{
	UINT nClientMajVersion = (dwTagVersionInfo >> 17) & 0x7f;
	UINT nClientMinVersion = (dwTagVersionInfo>> 10) & 0x7f;
	UINT nClientUpVersion  = (dwTagVersionInfo >>  7) & 0x07;
	CClientVersionInfo(nClientMajVersion, nClientMinVersion, nClientUpVersion, (UINT)CVI_IGNORED, nClientMajor, SO_UNKNOWN);
}

Constructor is called, new object is created and immediately destroyed. No assignment to the fields in the current object done.

While it is possible to call constructor as a method, it is considered as bad practice - and potentially dangerous.
A better way is to define privite initializer function, and the code could be rewritten about like this:

public:
	CClientVersionInfo(CString strPCEncodedVersion) {
		init((UINT)CVI_IGNORED, (UINT)CVI_IGNORED, (UINT)CVI_IGNORED, (UINT)CVI_IGNORED, SO_UNKNOWN, SO_UNKNOWN);

...

	CClientVersionInfo(uint32 dwTagVersionInfo, UINT nClientMajor) {
		UINT nClientMajVersion = (dwTagVersionInfo >> 17) & 0x7f;
		UINT nClientMinVersion = (dwTagVersionInfo>> 10) & 0x7f;
		UINT nClientUpVersion = (dwTagVersionInfo >>  7) & 0x07;
		init(nClientMajVersion, nClientMinVersion, nClientUpVersion, (UINT)CVI_IGNORED, nClientMajor, SO_UNKNOWN);
	}

	CClientVersionInfo(uint32 nVerMajor, uint32 nVerMinor, uint32 nVerUpdate, uint32 nVerBuild, uint32 ClientTypeMajor, uint32 ClientTypeMinor = SO_UNKNOWN) {
		init(nVerMajor, nVerMinor, nVerUpdate, nVerBuild, ClientTypeMajor, ClientTypeMinor);
	}

	CClientVersionInfo() {
		init((UINT)CVI_IGNORED, (UINT)CVI_IGNORED, (UINT)CVI_IGNORED, (UINT)CVI_IGNORED, SO_UNKNOWN, SO_UNKNOWN);
	}

	CClientVersionInfo(const CClientVersionInfo& cv) {
		*this = cv; 
	}

	CClientVersionInfo& operator=(const CClientVersionInfo& cv) {
		init(cv.m_nVerMajor, cv.m_nVerMinor, cv.m_nVerUpdate, cv.m_nVerBuild, cv.m_ClientTypeMajor, cv.m_ClientTypeMinor);
		return *this;
	}

...

private:
	void init(uint32 nVerMajor, uint32 nVerMinor, uint32 nVerUpdate, uint32 nVerBuild, uint32 ClientTypeMajor, uint32 ClientTypeMinor) {
		m_nVerMajor = nVerMajor;
		m_nVerMinor = nVerMinor;
		m_nVerUpdate = nVerUpdate;
		m_nVerBuild = nVerBuild;
		m_ClientTypeMajor = ClientTypeMajor;
		m_ClientTypeMinor = ClientTypeMinor;
	}

This post has been edited by fox88: 27 June 2015 - 03:03 PM

[fox88]

Some Support, on 25 April 2015 - 06:11 PM, said:

I have updated the devbuild with a new version which changes the upload slot speeds, ranging from 3KBs (the old value) for 3 open slots to 50KB/s per slot for over 40 open slots. But I haven't really had the chance to test it yet while uploading, so any feedback would be nice, esp. with fast lines - also trying unlimited, limited and USS as max speed.

I used to set 1024KB/s limit with 10Mbit/s line; and had perfectly flat line in upload graph.
With the latest change it is very uneven upload.
Right now: there are only 20 slots, and about half of the slots accept data at rates below 10KB/s.
Consequently there are frequent drops in the graph; commonly to 800KB/s, sometimes to 400 and below - recent one was to approximately 250.
Average speed is about 950KB/s only.

The conclusion: current algorithm with small fixed number of slots behaves inadequately.

Edit.
Decreased slot limit to 25*1024. The upload graph again is very smooth horizontal line.

This post has been edited by fox88: 13 May 2015 - 07:10 PM

[fox88]

Function bool gotostring(CFile &file, const uchar *find, LONGLONG plen) in otherfunctions.cpp is looking unfinished and unreliable.
Here is an edited version with removed unnecessary variables and check for errors or end of file when reading file:

bool gotostring(CFile &file, const uchar *find, LONGLONG plen)
{
	LONGLONG j = 0;
	ULONGLONG len = file.GetLength();
	uchar temp;

	for (ULONGLONG i = file.GetPosition(); i < len && file.Read(&temp, 1) == 1; i++) {
		if (temp == find[j])
			j++;
		else if (temp == find[0])
			j = 1;
		else
			j = 0;
		if (j >= plen)
			return true;
	}
	return false;
}

File CBase64Coding.cpp, method BOOL CBase64Coding::Encode(const char * source, int len, char * destination_string) contains extra lines, unused by eMule.
Declaration of number_of_bytes_encoded, initialized with zero is enough; the assignments are not needed any more:

	BYTE byte_3 = 0;
/*
   DWORD number_of_bytes_encoded = (DWORD) ( (double) number_of_bytes_to_encode / (double) 0.75 ) + 1;

   // Now add in the CR/LF pairs, each line is truncated at 72 characters

   // 2000-05-12
   // Thanks go to Ilia Golubev (ilia@varicom.co.il) for finding a bug here.
   // I was using number_of_bytes_to_encode rather than number_of_bytes_encoded.

   number_of_bytes_encoded += (DWORD)( ( ( number_of_bytes_encoded / BASE64_NUMBER_OF_CHARACTERS_PER_LINE ) + 1 ) * 2 );
*/
	DWORD number_of_bytes_encoded = 0;

	char * destination = destination_string;

A numbe of bugs with unsigned values.

File resizablelayout.cpp, method BOOL CResizableLayout::NeedsRefresh(const CResizableLayout::LayoutInfo& layout, const CRect& rectOld, const CRect& rectNew)
Line

info.nMax -= __max(info.nPage-1, 0);

should be

if (info.nPage > 1)
	info.nMax -= info.nPage-1;

File tag_render.cpp, method size_t ID3_TagImpl::PaddingSize(size_t curSize) const
Line

  if ((this->GetPrependedBytes()-ID3_TagHeader::SIZE > 0) &&

should be

  if ((this->GetPrependedBytes() > ID3_TagHeader::SIZE) &&

File EncryptedStreamSocket.cpp, method int CEncryptedStreamSocket::SendNegotiatingData(const void* lpBuf, uint32 nBufLen, uint32 nStartCryptFromByte, bool bDelaySend)
Line

if (nBufLen - nStartCryptFromByte > 0)

should be

if (nBufLen > nStartCryptFromByte)

File CorruptionBlackBox.cpp, method
void CCorruptionBlackBox::TransferredData(uint64 nStartPos, uint64 nEndPos, const CUpDownClient* pSender)
two asserts should be fixed

ASSERT( nRelEndPos - m_aaRecords[nPart][i].m_nStartPos > 0);
...
ASSERT( m_aaRecords[nPart][i].m_nEndPos - nRelStartPos > 0);

as

ASSERT(nRelEndPos > m_aaRecords[nPart][i].m_nStartPos);
...
ASSERT(m_aaRecords[nPart][i].m_nEndPos > nRelStartPos);

And one more bug in ResizableLib
There must be logical and operation instead of logical or.

This post has been edited by fox88: 18 July 2015 - 10:57 AM

[Some Support]

fox88, on 11 May 2015 - 04:45 PM, said:

Some Support, on 25 April 2015 - 06:11 PM, said:

I have updated the devbuild with a new version which changes the upload slot speeds, ranging from 3KBs (the old value) for 3 open slots to 50KB/s per slot for over 40 open slots. But I haven't really had the chance to test it yet while uploading, so any feedback would be nice, esp. with fast lines - also trying unlimited, limited and USS as max speed.

I used to set 1024KB/s limit with 10Mbit/s line; and had perfetly flat line in upload graph.
With the latest change it is very uneven upload.
Right now: there are only 20 slots, and about half of the slots accept data at rates below 10KB/s.
Consequently there are frequent drops in the graph; commonly to 800KB/s, sometimes to 400 and below - recent one was to approximately 250.
Average speed is about 950KB/s only.

The conclusion: current algorithm with small fixed number of slots behaves inadequately.

Edit.
Decreased slot limit to 25*1024. The upload graph again is very smooth horizontal line.

Thanks for the report. It seems to be rather a bug in the uploadcode than a problem with the intention itself. At 1024KB/s there should be more slots opened than 20 and even with 20 slots it's unlikely that they wouldn't be able to fill 10MBit. I'll look into that. Did you set a capacity and a limit or did you use USS?

[fox88]

Some Support, on 13 May 2015 - 10:39 AM, said:

Did you set a capacity and a limit or did you use USS?

Capacity is set at 1200.
I tried to use USS previously, not with the latest changes; and I don't think there were any downloads.
At first it was quite good, upload speed went up by 100 KB/s, if not more.
But about two days later I check in the morning, and upload is steady at 700 KB/s only.
So I reverted to static limit again.

Now with USS speed is 1151-1160KB/s. I'll watch how it behaves.

This post has been edited by fox88: 13 May 2015 - 09:22 PM

[fox88]

USS keeps behaving badly.
In the evenings it was fine when upload was above 1000 KB/s, sometimes very close to the limit 1200.
The line is smooth; not straight as a ruler, but close to that and definitely nothing like sawtooth pattern.

But in the mornings, when downloaders are less numerous, things get worse.
One time it was below 200, and in 40 minutes grew up to ~400 only.
Next time upload limit was so low, that there were only 4 slots active, and ~50 peer in queue.
Both times setting static limit soon increased upload speed to the usual value of 1024.

As far as I heard, USS checks ping to selected IP(s) and tries to adjust upload limit accorgingly.
In morning time there are less peers, and upload might drop rather low even with static limit.
I believe USS "thinks" that speed drops as a result of high network load and decreases the limit (sometimes too much and without any real necessity).

[Some Support]

fox88, on 16 May 2015 - 02:58 PM, said:

USS keeps behaving badly.
In the evenings it was fine when upload was above 1000 KB/s, sometimes very close to the limit 1200.
The line is smooth; not straight as a ruler, but close to that and definitely nothing like sawtooth pattern.

But in the mornings, when downloaders are less numerous, things get worse.
One time it was below 200, and in 40 minutes grew up to ~400 only.
Next time upload limit was so low, that there were only 4 slots active, and ~50 peer in queue.
Both times setting static limit soon increased upload speed to the usual value of 1024.

As far as I heard, USS checks ping to selected IP(s) and tries to adjust upload limit accorgingly.
In morning time there are less peers, and upload might drop rather low even with static limit.
I believe USS "thinks" that speed drops as a result of high network load and decreases the limit (sometimes too much and without any real necessity).

Is this behaviour new in build 16 or does it happen with other versions too?

[fox88]

Some Support, on 16 May 2015 - 05:43 PM, said:

Is this behaviour new in build 16 or does it happen with other versions too?

As I mentioned earlier, drop to 700 happened a while ago, before the latest changes.

Edit. That was with 3 KB/s slots.
Now I keep it down to 25 KB/s, not 50.

This post has been edited by fox88: 16 May 2015 - 05:28 PM