Official eMule-Board: Optimizations (discussion Thread) - Official eMule-Board

Jump to content


  • (2 Pages)
  • +
  • 1
  • 2

Optimizations (discussion Thread) Let's talk about it...

#1 User is offline   tHeWiZaRdOfDoS 

  • Man, what a bunch of jokers...
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 5630
  • Joined: 28-December 02

Post icon  Posted 02 January 2007 - 09:25 PM

Hi there!
I thought such a thread wouldn't be the worst idea... :angelnot:

PLEASE POST YOUR FINAL CODE Good point Ciccio... just started a proper thread HERE

@all: please do not spam this thread! I'll address a moderator and request a cleaning, keep it with the problems and code.


In UploadClient.cpp we do these checks:
// check extension to decide whether to compress or not
			CString ext = srcfile->GetFileName();
			ext.MakeLower();
			int pos = ext.ReverseFind(_T('.'));
			if (pos>-1)
				ext = ext.Mid(pos);
			bool compFlag = (ext!=_T(".zip") && ext!=_T(".cbz") && ext!=_T(".rar") && ext!=_T(".cbr") && ext!=_T(".ace") && ext!=_T(".ogm"));
			if (ext==_T(".avi") && thePrefs.GetDontCompressAvi())
				compFlag=false;

			if (!IsUploadingToPeerCache() && m_byDataCompVer == 1 && compFlag)
				CreatePackedPackets(filedata,togo,currentblock,bFromPF);
			else
				CreateStandartPackets(filedata,togo,currentblock,bFromPF);


But we can skip the expensive extension checks straight away using that check:
//>>> WiZaRd::Optimization
			bool compFlag = false;
			if (!IsUploadingToPeerCache() && m_byDataCompVer == 1)
			{
//<<< WiZaRd::Optimization
				// check extension to decide whether to compress or not
				CString ext = srcfile->GetFileName();
				ext.MakeLower();
				int pos = ext.ReverseFind(_T('.'));
				if (pos>-1)
					ext = ext.Mid(pos);
//>>> WiZaRd::Optimization
				compFlag = (ext!=_T(".zip") && ext!=_T(".cbz") && ext!=_T(".rar") && ext!=_T(".cbr") && ext!=_T(".ace") && ext!=_T(".ogm"));
//				bool compFlag = (ext!=_T(".zip") && ext!=_T(".cbz") && ext!=_T(".rar") && ext!=_T(".cbr") && ext!=_T(".ace") && ext!=_T(".ogm"));
//<<< WiZaRd::Optimization
				if (ext==_T(".avi") && thePrefs.GetDontCompressAvi())
					compFlag=false;
//>>> WiZaRd::Optimization
			}
			if (compFlag)
//			if (!IsUploadingToPeerCache() && m_byDataCompVer == 1 && compFlag)
//<<< WiZaRd::Optimization
				CreatePackedPackets(filedata,togo,currentblock,bFromPF);
			else
				CreateStandartPackets(filedata,togo,currentblock,bFromPF);



------------------------------------------------



In BaseClient.cpp we use this code:
bool CUpDownClient::TryToConnect(bool bIgnoreMaxCon, CRuntimeClass* pClassSocket)
{
	if (theApp.listensocket->TooManySockets() && !bIgnoreMaxCon && !(socket && socket->IsConnected()))
	{
		if(Disconnected(_T("Too many connections")))
		{
			delete this;
			return false;
		}
		return true;
	}


And we can again skip the rather expensive check in listensocket using that code:
bool CUpDownClient::TryToConnect(bool bIgnoreMaxCon, CRuntimeClass* pClassSocket)
{
//>>> WiZaRd::Optimization
	if (!bIgnoreMaxCon && !(socket && socket->IsConnected()) && theApp.listensocket->TooManySockets())
//	if (theApp.listensocket->TooManySockets() && !bIgnoreMaxCon && !(socket && socket->IsConnected()))
//<<< WiZaRd::Optimization
	{
		if(Disconnected(_T("Too many connections")))
		{
			delete this;
			return false;
		}
		return true;
	}




GreetZ,
WiZ

This post has been edited by tHeWiZaRdOfDoS: 08 January 2007 - 11:47 AM

0

#2 User is offline   eklmn 

  • Splendid Member
  • PipPipPipPip
  • Group: Member_D
  • Posts: 232
  • Joined: 12-January 03

Posted 02 January 2007 - 11:41 PM

the better solution will be an extension check in CKnownFile::SetFileName()...
0

#3 User is offline   Xman1 

  • Xtreme Modder
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 1955
  • Joined: 21-June 03

Posted 03 January 2007 - 08:18 AM

Quote

the better solution will be an extension check in CKnownFile::SetFileName()...


like at Xtreme... you can search for the tag "//Xman Code Improvement for choosing to use compression" at my sources.
0

#4 User is offline   tHeWiZaRdOfDoS 

  • Man, what a bunch of jokers...
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 5630
  • Joined: 28-December 02

Posted 03 January 2007 - 08:28 AM

Which needs a lot of re-writing again... I don't really like that idea... just wanted to start a thread including easy to apply fixes of unnecessary complicated codes :flowers:
0

#5 User is offline   BlueSonicBoy 

  • Magnificent Member
  • PipPipPipPipPipPip
  • Group: Members
  • Posts: 396
  • Joined: 26-September 04

Posted 03 January 2007 - 06:26 PM

View PosttHeWiZaRdOfDoS, on Jan 2 2007, 04:25 PM, said:

Hi there!
I thought such a thread wouldn't be the worst idea... :angelnot:


Great Idea! :worthy:
It may get a little confusing if people are discussing several posts at once... :flowers:

Here are a couple of tweaks for CIPFilterDlg

Below is the standard InitIPFilters()
[color="#000000"]void CIPFilterDlg::InitIPFilters()
{
	CWaitCursor curWait;

	m_uIPFilterItems = 0;
	free(m_ppIPFilterItems);
	m_ppIPFilterItems = NULL;

	[color="#0000FF"]const[/color] CIPFilterArray& ipfilter = theApp.ipfilter->GetIPFilter();
	m_uIPFilterItems = ipfilter.GetCount();
	m_ppIPFilterItems = (const SIPFilter**)malloc([color="#0000FF"]sizeof[/color](*m_ppIPFilterItems) * m_uIPFilterItems);

	m_ulFilteredIPs = 0;
	[color="#0000FF"]for[/color] (UINT i = 0; i < m_uIPFilterItems; i++)
	{
		[color="#0000FF"]const[/color] SIPFilter* pFilter = ipfilter[i];
		m_ppIPFilterItems[i] = pFilter;
		m_ulFilteredIPs += pFilter->end - pFilter->start + 1;
	}
	SortIPFilterItems();
	m_ipfilter.SetItemCount(m_uIPFilterItems);
	SetDlgItemText(IDC_TOTAL_IPFILTER, GetFormatedUInt(m_uIPFilterItems));
	SetDlgItemText(IDC_TOTAL_IPS, GetFormatedUInt(m_ulFilteredIPs));
}[/color]


Below is an optimized version..(I hope? :ph34r: )
[color="#000000"]void CIPFilterDlg::InitIPFilters()
{
	CWaitCursor curWait;

	m_uIPFilterItems = 0;
	m_ulFilteredIPs  = 0;
	free(m_ppIPFilterItems);
	m_ppIPFilterItems = NULL;

	[color="#0000FF"]const[/color] CIPFilterArray& ipfilter = theApp.ipfilter->GetIPFilter();
	m_uIPFilterItems = ipfilter.GetCount();
	[color="#0000FF"]void[/color]* pSource = ([color="#0000FF"]void[/color]*)ipfilter.GetData();
	size_t allocsize = [color="#0000FF"]sizeof[/color](*m_ppIPFilterItems) * m_uIPFilterItems;
	m_ppIPFilterItems= (const SIPFilter**) malloc(allocsize);	
	[color="#008000"]//copy all the data at once[/color]
	memcpy(([color="#0000FF"]void[/color]*)m_ppIPFilterItems,pSource,allocsize);
	UINT i = 0;
	for ( ; i < m_uIPFilterItems; i++)
	{
		const SIPFilter* pFilter = ipfilter[i];
		m_ulFilteredIPs += pFilter->end - pFilter->start;[color="#008000"]//+1[/color]
	}
	m_ulFilteredIPs += i;[color="#008000"]//one addition instead of i*additions[/color]
	SortIPFilterItems();
	m_ipfilter.SetItemCount(m_uIPFilterItems);
	SetDlgItemText(IDC_TOTAL_IPFILTER, GetFormatedUInt(m_uIPFilterItems));
	SetDlgItemText(IDC_TOTAL_IPS, GetFormatedUInt(m_ulFilteredIPs));
}[/color]


Below is the standard OnLvnGetDispInfoIPFilter()
[color="#000000"][color="#0000FF"]void[/color] CIPFilterDlg::OnLvnGetDispInfoIPFilter(NMHDR *pNMHDR, LRESULT *pResult)
{
	NMLVDISPINFO* pDispInfo = reinterpret_cast<NMLVDISPINFO*>(pNMHDR);
	if (pDispInfo->item.mask & LVIF_TEXT) [color="#008000"]// *have* to check that flag!![/color]
	{
		[color="#0000FF"]switch[/color] (pDispInfo->item.iSubItem)
		{
			[color="#0000FF"]case[/color] IPFILTER_COL_START:
				if (pDispInfo->item.cchTextMax > 0){
					_tcsncpy(pDispInfo->item.pszText, ipstr(htonl(m_ppIPFilterItems[pDispInfo->item.iItem]->start)), pDispInfo->item.cchTextMax);
					pDispInfo->item.pszText[pDispInfo->item.cchTextMax - 1] = _T('\');
				}
				[color="#0000FF"]break[/color];
			[color="#0000FF"]case[/color] IPFILTER_COL_END:
				if (pDispInfo->item.cchTextMax > 0){
					_tcsncpy(pDispInfo->item.pszText, ipstr(htonl(m_ppIPFilterItems[pDispInfo->item.iItem]->end)), pDispInfo->item.cchTextMax);
					pDispInfo->item.pszText[pDispInfo->item.cchTextMax - 1] = _T('\');
				}
				[color="#0000FF"]break[/color];
			[color="#0000FF"]case[/color] IPFILTER_COL_LEVEL:
				if (pDispInfo->item.cchTextMax > 0){
					_tcsncpy(pDispInfo->item.pszText, _itot(m_ppIPFilterItems[pDispInfo->item.iItem]->level, pDispInfo->item.pszText, 10), pDispInfo->item.cchTextMax);
					pDispInfo->item.pszText[pDispInfo->item.cchTextMax - 1] = _T('\');
				}
				[color="#0000FF"]break[/color];
			[color="#0000FF"]case[/color] IPFILTER_COL_HITS:
				if (pDispInfo->item.cchTextMax > 0){
					_tcsncpy(pDispInfo->item.pszText, _itot(m_ppIPFilterItems[pDispInfo->item.iItem]->hits, pDispInfo->item.pszText, 10), pDispInfo->item.cchTextMax);
					pDispInfo->item.pszText[pDispInfo->item.cchTextMax - 1] = _T('\');
				}
				[color="#0000FF"]break[/color];
			[color="#0000FF"]case[/color] IPFILTER_COL_DESC:
				if (pDispInfo->item.cchTextMax > 0){
					USES_CONVERSION;
					_tcsncpy(pDispInfo->item.pszText, A2T(m_ppIPFilterItems[pDispInfo->item.iItem]->desc), pDispInfo->item.cchTextMax);
					pDispInfo->item.pszText[pDispInfo->item.cchTextMax - 1] = _T('\');
				}
				[color="#0000FF"]break[/color];
		}
	}
	*pResult = 0;
}[/color]


Below is the optimized version. Note: pDispInfo->item.cchTextMax was returning a value of 260.
[color="#000000"]void CIPFilterDlg::OnLvnGetDispInfoIPFilter(NMHDR *pNMHDR, LRESULT *pResult)
{
	NMLVDISPINFO* pDispInfo = reinterpret_cast<NMLVDISPINFO*>(pNMHDR);
	if (pDispInfo->item.mask & LVIF_TEXT) [color="#008000"]// *have* to check that flag!![/color]
	{
		switch (pDispInfo->item.iSubItem)
		{
			[color="#0000FF"]case[/color] IPFILTER_COL_START:
				if (pDispInfo->item.cchTextMax >= 16){ [b][color="#008000"]//saves _tcsncpy padding with NULL characters[/color][/b]
					_tcsncpy(pDispInfo->item.pszText, ipstr(htonl(m_ppIPFilterItems[pDispInfo->item.iItem]->start)), 15);
					pDispInfo->item.pszText[15] = _T('\');
				}
				[color="#0000FF"]break[/color];
			[color="#0000FF"]case[/color] IPFILTER_COL_END:
				if (pDispInfo->item.cchTextMax >= 16){
					_tcsncpy(pDispInfo->item.pszText, ipstr(htonl(m_ppIPFilterItems[pDispInfo->item.iItem]->end)), 15);
					pDispInfo->item.pszText[15] = _T('\');
				}
				[color="#0000FF"]break[/color];
			[color="#0000FF"]case[/color] IPFILTER_COL_LEVEL:
				if (pDispInfo->item.cchTextMax >= 5){
					_tcsncpy(pDispInfo->item.pszText, _itot(m_ppIPFilterItems[pDispInfo->item.iItem]->level, pDispInfo->item.pszText, 10), 5);
					pDispInfo->item.pszText[5] = _T('\');
				}
				[color="#0000FF"]break[/color];
			[color="#0000FF"]case[/color] IPFILTER_COL_HITS:
				if (pDispInfo->item.cchTextMax >= 5){
					_tcsncpy(pDispInfo->item.pszText, _itot(m_ppIPFilterItems[pDispInfo->item.iItem]->hits, pDispInfo->item.pszText, 10), 5);
					pDispInfo->item.pszText[5] = _T('\');
				}
				[color="#0000FF"]break[/color];
			[color="#0000FF"]case[/color] IPFILTER_COL_DESC:
				if(pDispInfo->item.cchTextMax > 0)
				  {
				    int stringlength = m_ppIPFilterItems[pDispInfo->item.iItem]->desc.GetLength();
				    if(stringlength > pDispInfo->item.cchTextMax) stringlength = pDispInfo->item.cchTextMax;
				    USES_CONVERSION;
				    _tcsncpy(pDispInfo->item.pszText, A2T(m_ppIPFilterItems[pDispInfo->item.iItem]->desc), stringlength);
					pDispInfo->item.pszText[stringlength] = _T('\');
				  }
				[color="#0000FF"]break[/color];
		}
	}
	*pResult = 0;
}
[/color]


Thanks for reading. :flowers:
0

#6 User is offline   tHeWiZaRdOfDoS 

  • Man, what a bunch of jokers...
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 5630
  • Joined: 28-December 02

Posted 03 January 2007 - 09:02 PM

@BSB: It'd be good if you would tag your changes so they are easier to find...
The "optimization" in the ini saves one muliplication but as we have to parse the loop anyways I can't see any use otherwise... concerning the display tweak - why do you cut it to at least 16 chars length? The IP might have only (might!) 8 chars (0.0.0.0.)
0

#7 User is offline   erpirata 

  • Member
  • PipPip
  • Group: Members
  • Posts: 48
  • Joined: 28-December 06

Posted 04 January 2007 - 12:15 AM

BlueSonicBoy in your code I have found some errors and I have corrected them:
1>.\IPFilterDlg.cpp(275) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(281) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(287) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(293) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(303) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(275) : fatal error C1057: unexpected end of file in macro expansion


line 275
pDispInfo->item.pszText[15] = _T('\');
to change in
pDispInfo->item.pszText[15] = _T('\O');

line 281
pDispInfo->item.pszText[15]= _T('\');
to change in
pDispInfo->item.pszText[15]= _T('\O');

line 287
pDispInfo->item.pszText[5]= _T('\');
to change in
pDispInfo->item.pszText[5]= _T('\O');

line 293
pDispInfo->item.pszText[5]= _T('\');
to change in
pDispInfo->item.pszText[5]= _T('\O');

line 303
pDispInfo->item.pszText[stringlength]= _T('\');
to change in
pDispInfo->item.pszText[stringlength]= _T('\O');

p.s.
\O = \zero

This post has been edited by erpirata: 04 January 2007 - 12:59 AM

0

#8 User is offline   BlueSonicBoy 

  • Magnificent Member
  • PipPipPipPipPipPip
  • Group: Members
  • Posts: 396
  • Joined: 26-September 04

Posted 04 January 2007 - 02:14 AM

View PosttHeWiZaRdOfDoS, on Jan 3 2007, 04:02 PM, said:

The "optimization" in the ini saves one muliplication but as we have to parse the loop anyways I can't see any use otherwise...



In the original code we had m_ulFilteredIPs += pFilter->end - pFilter->start + 1; carried out i times. Where i is the number of filtered IP ranges, which tends to be a lot nowadays...

If the compiler doesn't do anything clever, that means a wasted +1 every loop, even if that were compiled as a register increment per loop, that's 2? clock cycles * i (i currently could equal 136674)...

Whereas adding i to the final value after the loop only costs one addition.

The second optimize I can't prove is faster, but intuitively it should be...
In the original code we do this m_ppIPFilterItems[i] = pFilter; copying one IP Filter entery in to our newly created memory buffer then looping and eventualy copying the next.
My code copies the whole thing into the new memory buffer in on go using memcpy((void*)m_ppIPFilterItems,pSource,allocsize);




View PosttHeWiZaRdOfDoS, on Jan 3 2007, 04:02 PM, said:

concerning the display tweak - why do you cut it to at least 16 chars length? The IP might have only (might!) 8 chars (0.0.0.0.)


Because the most it will have is 15 + the NULL character. :flowers:
Worst case, _tcsncpy(pDispInfo->item.pszText, ipstr(htonl(m_ppIPFilterItems[pDispInfo->item.iItem]->end)), 15);
will pad pDispInfo->item.pszText with 7 NULL characters rather than 252 for the original code. :flowers:
0

#9 User is offline   BlueSonicBoy 

  • Magnificent Member
  • PipPipPipPipPipPip
  • Group: Members
  • Posts: 396
  • Joined: 26-September 04

Posted 04 January 2007 - 02:33 AM

View Posterpirata, on Jan 3 2007, 07:15 PM, said:

BlueSonicBoy in your code I have found some errors and I have corrected them:
1>.\IPFilterDlg.cpp(275) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(281) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(287) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(293) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(303) : error C2001: newline in constant
1>.\IPFilterDlg.cpp(275) : fatal error C1057: unexpected end of file in macro expansion


line 303
pDispInfo->item.pszText[stringlength]= _T('\');
to change in
pDispInfo->item.pszText[stringlength]= _T('\O');

p.s.
\O = \zero


Thank you!! :flowers:

In the code I posted it was as you have written,(see my Mod's source); but the CODEBOX loses the 0! What is the point of a CODEBOX that alters your code? Surely any characters should be safe in the code box?? :confused: :furious:
0

#10 User is offline   erpirata 

  • Member
  • PipPip
  • Group: Members
  • Posts: 48
  • Joined: 28-December 06

Posted 04 January 2007 - 03:54 AM

I confirm that it is impossible to write \O with the 0 in place of the O, in this FORUM. :shock: :shock:
0

#11 User is offline   tHeWiZaRdOfDoS 

  • Man, what a bunch of jokers...
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 5630
  • Joined: 28-December 02

Post icon  Posted 05 January 2007 - 10:52 AM

Here is a rather expensive one...

Code in PartFile.cpp:
// green progress
		float blockpixel = (float)(rect->right - rect->left)/(float)m_nFileSize;
		RECT gaprect;
		gaprect.top = rect->top;
		gaprect.bottom = gaprect.top + PROGRESS_HEIGHT;
		gaprect.left = rect->left;
	
		if (!bFlat) {
			s_LoadBar.SetWidth((int)( (uint64)(m_nFileSize - allgaps)*blockpixel + 0.5F));
			s_LoadBar.Fill(crProgress);
			s_LoadBar.Draw(dc, gaprect.left, gaprect.top, false);
		} else {
			gaprect.right = rect->left + (uint32)((uint64)(m_nFileSize - allgaps)*blockpixel + 0.5F);
			dc->FillRect(&gaprect, &CBrush(crProgress));
			//draw gray progress only if flat
			gaprect.left = gaprect.right;
			gaprect.right = rect->right;
			dc->FillRect(&gaprect, &CBrush(crProgressBk));
		}


Should be:

// green progress
//>>> WiZaRd::Optimization
		if(GetFileOp() == PFOP_NONE)
		{
//<<< WiZaRd::Optimization
			float blockpixel = (float)(rect->right - rect->left)/(float)m_nFileSize;
			RECT gaprect;
			gaprect.top = rect->top;
			gaprect.bottom = gaprect.top + PROGRESS_HEIGHT;
			gaprect.left = rect->left;
		
			if (!bFlat) {
				s_LoadBar.SetWidth((int)( (uint64)(m_nFileSize - allgaps)*blockpixel + 0.5F));
				s_LoadBar.Fill(crProgress);
				s_LoadBar.Draw(dc, gaprect.left, gaprect.top, false);
			} else {
				gaprect.right = rect->left + (uint32)((uint64)(m_nFileSize - allgaps)*blockpixel + 0.5F);
				dc->FillRect(&gaprect, &CBrush(crProgress));
				//draw gray progress only if flat
				gaprect.left = gaprect.right;
				gaprect.right = rect->right;
				dc->FillRect(&gaprect, &CBrush(crProgressBk));
			}
//>>> WiZaRd::Optimization
		}
//<<< WiZaRd::Optimization


because the other cases will overwrite the loadbar anyways... of course it could also be added below:

s_ChunkBar.Fill(crHave);

	uint64 allgaps = 0; //>>> WiZaRd Optimization
	if (status == PS_COMPLETE || status == PS_COMPLETING)
	{
		s_ChunkBar.FillRange(0, m_nFileSize, crProgress);
		s_ChunkBar.Draw(dc, rect->left, rect->top, bFlat);
		percentcompleted = 100.0F;
		completedsize = m_nFileSize;
	}
	else if (theApp.m_brushBackwardDiagonal.m_hObject && eVirtualState == PS_INSUFFICIENT || status == PS_ERROR)
	{
		int iOldBkColor = dc->SetBkColor(RGB(255, 255, 0));
		dc->FillRect(rect, &theApp.m_brushBackwardDiagonal);
		dc->SetBkColor(iOldBkColor);

		UpdateCompletedInfos();
	}
	else
	{
		// red gaps
//		uint64 allgaps = 0; //>>> WiZaRd Optimization
...
...
// additionally show any file op progress (needed for PS_COMPLETING and PS_WAITINGFORHASH)
//>>> WiZaRd::Optimization
	if(GetFileOp() == PFOP_NONE)
	{
		float blockpixel = (float)(rect->right - rect->left)/(float)m_nFileSize;
		RECT gaprect;
		gaprect.top = rect->top;
		gaprect.bottom = gaprect.top + PROGRESS_HEIGHT;
		gaprect.left = rect->left;

		if (!bFlat) {
			s_LoadBar.SetWidth((int)( (uint64)(m_nFileSize - allgaps)*blockpixel + 0.5F));
			s_LoadBar.Fill(crProgress);
			s_LoadBar.Draw(dc, gaprect.left, gaprect.top, false);
		} else {
			gaprect.right = rect->left + (uint32)((uint64)(m_nFileSize - allgaps)*blockpixel + 0.5F);
			dc->FillRect(&gaprect, &CBrush(crProgress));
			//draw gray progress only if flat
			gaprect.left = gaprect.right;
			gaprect.right = rect->right;
			dc->FillRect(&gaprect, &CBrush(crProgressBk));
		}
	}
	else
	//if (GetFileOp() != PFOP_NONE)
//<<< WiZaRd::Optimization	
	{




Quote

Because the most it will have is 15 + the NULL character.
Worst case, _tcsncpy(pDispInfo->item.pszText, ipstr(htonl(m_ppIPFilterItems[pDispInfo->item.iItem]->end)), 15);
will pad pDispInfo->item.pszText with 7 NULL characters rather than 252 for the original code.

Well, wouldn't it be even better to use something like:

if (pDispInfo->item.cchTextMax > 0){
					CString ip = ipstr(htonl(m_ppIPFilterItems[pDispInfo->item.iItem]->start));
					size_t cut = min(ip.GetLength(), pDispInfo->item.cchTextMax);
					_tcsncpy(pDispInfo->item.pszText, ip, cut);
					pDispInfo->item.pszText[cut - 1] = _T('\');
				}


GreetZ,
WiZ

This post has been edited by tHeWiZaRdOfDoS: 05 January 2007 - 03:56 PM

0

#12 User is offline   erpirata 

  • Member
  • PipPip
  • Group: Members
  • Posts: 48
  • Joined: 28-December 06

Posted 05 January 2007 - 12:59 PM

my opininion the modification of BlueSonicBoy is incomplete because it does not consider two file "DownloadListCtrl.cpp" and "SharedFilesCtrl.cpp". Also they use the same function, when it has only modified IPFilterDlg.cpp.

Original DownloadListCtrl.cpp
if (pDispInfo->item.cchTextMax > O){
							_tcsncpy(pDispInfo->item.pszText, ((CPartFile*)pItem->value)->GetFileName(), pDispInfo->item.cchTextMax);
							pDispInfo->item.pszText[pDispInfo->item.cchTextMax-1] = _T('\O');


Original SharedFilesCtrl.cpp
if (pDispInfo->item.cchTextMax > O){
							_tcsncpy(pDispInfo->item.pszText, pFile->GetFileName(), pDispInfo->item.cchTextMax);
							pDispInfo->item.pszText[pDispInfo->item.cchTextMax-1] = _T('\O');


for tHeWiZaRdOfDoS

Quote

1>.\PartFile.cpp(2278) : error C2065: 'allgaps' : undeclared identifier


line 2278
s_LoadBar.SetWidth((int)( (uint64)(m_nFileSize - allgaps)*blockpixel + 0.5F));

0

#13 User is offline   tHeWiZaRdOfDoS 

  • Man, what a bunch of jokers...
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 5630
  • Joined: 28-December 02

Posted 05 January 2007 - 03:58 PM

@erpirata: there is an "edit" button you know? fixed your problem above! thx for testing...
@all: please do not spam this thread! I'll address a moderator and request a cleaning, keep it with the problems and code.
0

#14 User is offline   BlueSonicBoy 

  • Magnificent Member
  • PipPipPipPipPipPip
  • Group: Members
  • Posts: 396
  • Joined: 26-September 04

Posted 06 January 2007 - 12:50 AM

View PosttHeWiZaRdOfDoS, on Jan 5 2007, 05:52 AM, said:

Quote

Because the most it will have is 15 + the NULL character.
Worst case, _tcsncpy(pDispInfo->item.pszText, ipstr(htonl(m_ppIPFilterItems[pDispInfo->item.iItem]->end)), 15);
will pad pDispInfo->item.pszText with 7 NULL characters rather than 252 for the original code.

Well, wouldn't it be even better to use something like:

if (pDispInfo->item.cchTextMax > 0){
					CString ip = ipstr(htonl(m_ppIPFilterItems[pDispInfo->item.iItem]->start));
					size_t cut = min(ip.GetLength(), pDispInfo->item.cchTextMax);
					_tcsncpy(pDispInfo->item.pszText, ip, cut);
					pDispInfo->item.pszText[cut - 1] = _T('\');
				}


Thanks for the void CPartFile::DrawStatusBar() Optimize.

I am unsure whether size_t cut = min(ip.GetLength(), pDispInfo->item.cchTextMax); would execute faster than _tcsncpy() would add 7 NULL characters... But your solution does looks more elegant. :flowers: :flowers:
0

#15 User is offline   Enig123 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 553
  • Joined: 22-November 04

Posted 07 January 2007 - 03:39 AM

WiZaRd

In PartFile.cpp the optimization taken have a tiny bug, which is that unlike usual behavior, the progress bar stuck at 100% and won't disappear even after finish hashing.

Any step that can fix this?

This post has been edited by Enig123: 07 January 2007 - 08:12 AM

0

#16 User is offline   BlueSonicBoy 

  • Magnificent Member
  • PipPipPipPipPipPip
  • Group: Members
  • Posts: 396
  • Joined: 26-September 04

Post icon  Posted 07 January 2007 - 04:40 AM

@WiZaRd: This thread was a great idea! :thumbup:


I was doing some profiling... B) When I noted that in CClientUDPSocket::SendTo() ipstr() took what I believed to be an excessive execution time for what it did. So, I looked at the code,(see the current code below) ipstr() returns a CString object and for eMule that's CStringT. Now CString objects seem expensive to me anyway, but when I looked at CAsyncSocket::CAsyncSocket::SendTo(const void* lpBuf, int nBufLen, UINT nHostPort, LPCTSTR lpszHostAddress, int nFlags) I found the first thing it does to our newly created TChar CString is convert it to an ASCII string, it then takes that ASCII string and with inet_addr(lpszAscii); turns it back to the uint32 value we started with in dwIP!! Finally it makes a call to SendTo(lpBuf, nBufLen, (SOCKADDR*)&sockAddr, sizeof(sockAddr), nFlags);

Original CClientUDPSocket::SendTo()
[color="#000000"][color="#0000FF"]int[/color] CClientUDPSocket::SendTo([color="#0000FF"]char[/color]* lpBuf,[color="#0000FF"]int [/color]nBufLen,uint32 dwIP, uint16 nPort){
	[color="#008000"]// NOTE: *** This function is invoked from a *different* thread![/color]
	uint32 result = CAsyncSocket::SendTo(lpBuf,nBufLen,nPort,[b]ipstr(dwIP)[/b]);
	[color="#0000FF"]if[/color] (result == (uint32)SOCKET_ERROR){
		uint32 error = GetLastError();
		if (error == WSAEWOULDBLOCK){
			m_bWouldBlock = [color="#0000FF"]true[/color];
			[color="#0000FF"]return[/color] -1;
		}
		[color="#0000FF"]if[/color] (thePrefs.GetVerbose())
			DebugLogError(_T("Error: Client UDP socket, failed to send data to %s:%u: %s"), ipstr(dwIP), nPort, GetErrorMessage(error, 1));
	}
	[color="#0000FF"]return[/color] 0;[/color]
}

MFC CAsyncSocket::CAsyncSocket::SendTo(const void* lpBuf, int nBufLen, UINT nHostPort, LPCTSTR lpszHostAddress, int nFlags)
[size=1][color="#000000"]
[color="#008000"]// This is a part of the Microsoft Foundation Classes C++ library.
// Copyright © Microsoft Corporation
// All rights reserved.
//
// This source code is only intended as a supplement to the
// Microsoft Foundation Classes Reference and related
// electronic documentation provided with the library.
// See these sources for detailed information regarding the
// Microsoft Foundation Classes product.[/color]
[color="#0000FF"]int[/color] CAsyncSocket::SendTo([color="#0000FF"]const void[/color]* lpBuf, [color="#0000FF"]int[/color] nBufLen, UINT nHostPort, LPCTSTR lpszHostAddress, [color="#0000FF"]int[/color] nFlags)
{
	USES_CONVERSION_EX;

	SOCKADDR_IN sockAddr;

	memset(&sockAddr,0,[color="#0000FF"]sizeof[/color](sockAddr));

	LPSTR lpszAscii;
	[color="#0000FF"]if[/color] (lpszHostAddress != NULL)
	{
		lpszAscii = T2A_EX((LPTSTR)lpszHostAddress, _ATL_SAFE_ALLOCA_DEF_THRESHOLD);
		[color="#0000FF"]if[/color] (lpszAscii == NULL)
		{
			[color="#008000"]// OUT OF MEMORY[/color]
			WSASetLastError(ERROR_NOT_ENOUGH_MEMORY);
			[color="#0000FF"]return[/color] FALSE;
		}	
	}
	[color="#0000FF"]else[/color]
	{
		lpszAscii = NULL;
	}

	sockAddr.sin_family = AF_INET;

	[color="#0000FF"]if[/color] (lpszAscii == NULL)
		sockAddr.sin_addr.s_addr = htonl(INADDR_BROADCAST);
	[color="#0000FF"]else[/color]
	{
		sockAddr.sin_addr.s_addr = inet_addr(lpszAscii);
		[color="#0000FF"]if[/color] (sockAddr.sin_addr.s_addr == INADDR_NONE)
		{
			LPHOSTENT lphost;
			lphost = gethostbyname(lpszAscii);
			[color="#0000FF"]if[/color] (lphost != NULL)
				sockAddr.sin_addr.s_addr = ((LPIN_ADDR)lphost->h_addr)->s_addr;
			[color="#0000FF"]else[/color]
			{
				WSASetLastError(WSAEINVAL);
				[color="#0000FF"]return[/color] SOCKET_ERROR;
			}
		}
	}

	sockAddr.sin_port = htons((u_short)nHostPort);

	[b][color="#0000FF"]return[/color] SendTo(lpBuf, nBufLen, (SOCKADDR*)&sockAddr, [color="#0000FF"]sizeof[/color](sockAddr), nFlags);[/b]
}[/color][/size]


Below is my optimized version of CClientUDPSocket::SendTo()
[color="#000000"][color="#0000FF"]int[/color] CClientUDPSocket::SendTo([color="#0000FF"]char[/color]* lpBuf,int nBufLen,uint32 dwIP, uint16 nPort){
	 [color="#008000"]// NOTE: *** This function is invoked from a *different* thread!
[b]	//TK4 MOD 2.0c - Save converting dwIP from uint32 to a CStringT to ASCII then back to uint32[/color]
	[color="#0000FF"]if[/color](dwIP == INADDR_BROADCAST)
	  {
            WSASetLastError(WSAEINVAL);
	    [color="#0000FF"]if[/color] (thePrefs.GetVerbose())
			DebugLogError(_T("Error: Client UDP socket, failed to send data to %s:%u: %s"), ipstr(dwIP), nPort, GetErrorMessage(WSAEINVAL, 1));
            [color="#0000FF"]return[/color] 0;
	   }

	SOCKADDR_IN sockAddr;
	memset(&sockAddr, 0,sizeof(sockAddr));
 	sockAddr.sin_family = AF_INET;
	sockAddr.sin_addr.s_addr = dwIP;
	sockAddr.sin_port = htons((u_short)nPort);

	uint32 result = CAsyncSocket::SendTo(lpBuf, nBufLen, (SOCKADDR*)&sockAddr, [color="#0000FF"]sizeof[/color](sockAddr), 0);[/b]

[color="#008000"]//	uint32 result = CAsyncSocket::SendTo(lpBuf,nBufLen,nPort,ipstr(dwIP)); [/color]

	[color="#0000FF"]if[/color] (result == (uint32)SOCKET_ERROR){
		uint32 error = GetLastError();
		[color="#0000FF"]if[/color] (error == WSAEWOULDBLOCK){
			m_bWouldBlock = [color="#0000FF"]true[/color];
			[color="#0000FF"]return[/color] -1;
		}
		[color="#0000FF"]if[/color] (thePrefs.GetVerbose())
			DebugLogError(_T("Error: Client UDP socket, failed to send data to %s:%u: %s"), ipstr(dwIP), nPort, GetErrorMessage(error, 1));
	}
	[color="#0000FF"]return[/color] 0;
}[/color]


The same optimize can be applied to CUDPSocket::SendTo()
The Optimized code is below
[color="#000000"][color="#0000FF"]int[/color] CUDPSocket::SendTo(BYTE* lpBuf, [color="#0000FF"]int[/color] nBufLen, uint32 dwIP, uint16 nPort)
{
	[color="#008000"]// NOTE: *** This function is invoked from a *different* thread!
	[b]//TK4 MOD 2.0c - Save converting dwIP from uint32 to a CStringT to ASCII then back to uint32[/color]
	[color="#0000FF"]if[/color](dwIP == INADDR_BROADCAST)
	  {
            WSASetLastError(WSAEINVAL);
	    [color="#0000FF"]if[/color] (thePrefs.GetVerbose())
			DebugLogError(_T("Error: Client UDP socket, failed to send data to %s:%u: %s"), ipstr(dwIP), nPort, GetErrorMessage(WSAEINVAL, 1));
            [color="#0000FF"]return[/color] 0;
	   }

	SOCKADDR_IN sockAddr;
	memset(&sockAddr, 0,[color="#0000FF"]sizeof[/color](sockAddr));
	sockAddr.sin_family = AF_INET;
	sockAddr.sin_addr.s_addr = dwIP;
	sockAddr.sin_port = htons((u_short)nPort);

	[color="#0000FF"]int[/color] iResult = CAsyncSocket::SendTo(lpBuf, nBufLen, (SOCKADDR*)&sockAddr, sizeof(sockAddr), 0);[/b]

[color="#008000"]	//int iResult = CAsyncSocket::SendTo(lpBuf, nBufLen, nPort, ipstr(dwIP));[/color]
	[color="#0000FF"]if[/color] (iResult == SOCKET_ERROR) {
		DWORD dwError = GetLastError();
		[color="#0000FF"]if[/color] (dwError == WSAEWOULDBLOCK) {
			m_bWouldBlock = [color="#0000FF"]true[/color];
			[color="#0000FF"]return[/color] -1; [color="#008000"]// blocked[/color]
		}
		[color="#0000FF"]else[/color]{
			[color="#0000FF"]if[/color] (thePrefs.GetVerbose())
				theApp.QueueDebugLogLine([color="#0000FF"]false[/color], _T("Error: Server UDP socket: Failed to send packet to %s:%u - %s"), ipstr(dwIP), nPort, GetErrorMessage(dwError, 1));
			[color="#0000FF"]return[/color] 0; [color="#008000"]// error[/color]
		}
	}
	[color="#0000FF"]return[/color] 1; [color="#008000"]// success[/color]
}[/color]


Thanks for reading! :flowers:
Fix Added! 07-01-2007 :angelnot:

This post has been edited by BlueSonicBoy: 07 January 2007 - 07:09 PM

0

#17 User is offline   Enig123 

  • Golden eMule
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 553
  • Joined: 22-November 04

Posted 07 January 2007 - 07:49 AM

BlueSonicBoy:

Wow, what a nice catch. :thumbup:
Already applied to my own compile without problem.
0

#18 User is offline   tHeWiZaRdOfDoS 

  • Man, what a bunch of jokers...
  • PipPipPipPipPipPipPip
  • Group: Members
  • Posts: 5630
  • Joined: 28-December 02

Posted 07 January 2007 - 10:31 AM

View PostEnig123, on Jan 7 2007, 04:39 AM, said:

In PartFile.cpp the optimization taken have a tiny bug, which is that unlike usual behavior, the progress bar stuck at 100% and won't disappear even after finish hashing.

Any step that can fix this?

Well in fact this is intended because the loadbar represents the progress and if the file is complete it should be filled :flowers:
However you can change the code for the latter patch that way to have the original behaviour:
uint64 allgaps = 0; //>>> WiZaRd Optimization
	bool bDraw = false; //>>> For Enig123
	if (status == PS_COMPLETE || status == PS_COMPLETING)
	{
		s_ChunkBar.FillRange(0, m_nFileSize, crProgress);
		s_ChunkBar.Draw(dc, rect->left, rect->top, bFlat);
		percentcompleted = 100.0F;
		completedsize = m_nFileSize;
	}
	else if (theApp.m_brushBackwardDiagonal.m_hObject && eVirtualState == PS_INSUFFICIENT || status == PS_ERROR)
	{
		int iOldBkColor = dc->SetBkColor(RGB(255, 255, 0));
		dc->FillRect(rect, &theApp.m_brushBackwardDiagonal);
		dc->SetBkColor(iOldBkColor);

		UpdateCompletedInfos();
	}
	else
	{
		bDraw = GetFileOp() == PFOP_NONE; //>>> For Enig123
...
	// additionally show any file op progress (needed for PS_COMPLETING and PS_WAITINGFORHASH)
//>>> WiZaRd::Optimization
//	if(GetFileOp() == PFOP_NONE) 
	if (bDraw) //>>> For Enig123



@BSB: that's right but what about the error handling included in the first part of the CAsyncSocket::SendTo? And shouldn't we at least apply a htonl call on the IP?

This post has been edited by tHeWiZaRdOfDoS: 07 January 2007 - 10:33 AM

0

#19 User is offline   BlueSonicBoy 

  • Magnificent Member
  • PipPipPipPipPipPip
  • Group: Members
  • Posts: 396
  • Joined: 26-September 04

Posted 07 January 2007 - 07:41 PM

View PosttHeWiZaRdOfDoS, on Jan 7 2007, 05:31 AM, said:

@BSB: that's right but what about the error handling included in the first part of the CAsyncSocket::SendTo?


Thanks WiZaRd!! I was just annotating CAsyncSocket::SendTo() and I noticed the INADDR_NONE error returned by
inet_addr(); has the value 0xffffffff which equals 255.255.255.255 the limited broadcast address INADDR_BROADCAST. I have alter the optimizes accordingly! :flowers:

MFC CAsyncSocket::SendTo(const void* lpBuf, int nBufLen, UINT nHostPort, LPCTSTR lpszHostAddress, int nFlags)
[size=1][color="#000000"]
[color="#008000"]// This is a part of the Microsoft Foundation Classes C++ library.
// Copyright © Microsoft Corporation
// All rights reserved.
//
// This source code is only intended as a supplement to the
// Microsoft Foundation Classes Reference and related
// electronic documentation provided with the library.
// See these sources for detailed information regarding the
// Microsoft Foundation Classes product.[/color]
[color="#0000FF"]int[/color] CAsyncSocket::SendTo([color="#0000FF"]const void[/color]* lpBuf, [color="#0000FF"]int[/color] nBufLen, UINT nHostPort, LPCTSTR lpszHostAddress, [color="#0000FF"]int[/color] nFlags)
{
	[i][color="#FF0000"]USES_CONVERSION_EX;[/color][/i]

	[b][color="#800080"]SOCKADDR_IN sockAddr;[/color][/b] [b]<- Retained[/b]

	[b][color="#800080"]memset(&sockAddr,0,[color="#0000FF"]sizeof[/color](sockAddr));[/color][/b] [b]<- Retained[/b]


	[i][color="#FF0000"]LPSTR lpszAscii;
	[color="#0000FF"]if[/color] (lpszHostAddress != NULL)
	{
		lpszAscii = T2A_EX((LPTSTR)lpszHostAddress, _ATL_SAFE_ALLOCA_DEF_THRESHOLD);
		[color="#0000FF"]if[/color] (lpszAscii == NULL)
		{
			[color="#008000"]// OUT OF MEMORY[/color]
			WSASetLastError(ERROR_NOT_ENOUGH_MEMORY);
			[color="#0000FF"]return[/color] FALSE;
		}	
	}
	[color="#0000FF"]else[/color]
	{
		lpszAscii = NULL;
	}
[/color][/i]

        [b]^^^^^ Above String conversion only       
[/b]
	[b][color="#800080"]sockAddr.sin_family = AF_INET;[/color][/b] [b]<- Retained[/b]

	[color="#0000FF"]if[/color] (lpszAscii == NULL)     [b]<- To deal with and empty string being passed, can never happen with the original code[/b]
		sockAddr.sin_addr.s_addr = htonl(INADDR_BROADCAST);[i] <--- Why htonl?? #define INADDR_BROADCAST        (u_long)0xffffffff [/i] :confused: 
	[color="#0000FF"]else[/color]
	{
		[b]sockAddr.sin_addr.s_addr = inet_addr(lpszAscii);[/b] [size=3][b]<--- ***THIS LINE Returns the value dwIP !!![/b][/size]
		[color="#0000FF"]if[/color] (sockAddr.sin_addr.s_addr == INADDR_NONE) [b]<--- Deals with an error returned by inet_addr() BUT also could occur if INADDR_BROADCAST address is passed!![/b]
		{
			[i]LPHOSTENT lphost;
			lphost = gethostbyname(lpszAscii);[/i] [b]<-- A check to see if a host name had been passed in the original string, not applicable to our code[/b]
			[color="#0000FF"]if[/color] (lphost != NULL)
				[i]sockAddr.sin_addr.s_addr = ((LPIN_ADDR)lphost->h_addr)->s_addr;[/i]
			[color="#0000FF"]else[/color]
			{                                   
				WSASetLastError(WSAEINVAL);                             [b]<-- Just added to the optimize for 255.255.255.255[/b]
				[color="#0000FF"]return[/color] SOCKET_ERROR; 
			}
		}
	}

	[b][color="#800080"]sockAddr.sin_port = htons((u_short)nHostPort);[/color][/b] [b]<--- Retained[/b]

	[b][color="#0000FF"]return[/color] SendTo(lpBuf, nBufLen, (SOCKADDR*)&sockAddr, [color="#0000FF"]sizeof[/color](sockAddr), nFlags);[/b] [b]<--- Call Retained[/b]
}[/color][/size]


View PosttHeWiZaRdOfDoS, on Jan 7 2007, 05:31 AM, said:

And shouldn't we at least apply a htonl call on the IP?


No, dwIP's byte order is correct already its value is the same as the value returned by inet_addr(lpszAscii);
Put a break point at sockAddr.sin_addr.s_addr = inet_addr(lpszAscii); and you will see. :flowers: :flowers:
0

#20 User is offline   CiccioBastardo 

  • Doomsday Executor
  • PipPipPipPipPipPipPip
  • Group: Italian Moderators
  • Posts: 5541
  • Joined: 22-November 03

Posted 07 January 2007 - 09:11 PM

Hi all,
for what it matters, I think the idea of single thread for discussing different and many code modifications is not that good. It will end in a mess, both for discussion and for finding the right code modification in the future. I propose to create a parallel thread to this one where the fixes exposed and discussed here are finally published. In that thread there won't be discussions, only the code of what is considered the latest good way to make the patch.
In such a thread each post would contain a single patch, where the problem it tries to solve, and which version or mod is destined to, is explained in a clear way.
The problem is not the client, it's the user
0

  • Member Options

  • (2 Pages)
  • +
  • 1
  • 2

1 User(s) are reading this topic
0 members, 1 guests, 0 anonymous users