From: Yurii Hordiienko <45203053+hordi@users.noreply.github.com>
Date: Sat, 19 Nov 2022 12:05:51 +0000 (-0500)
Subject: DiffFileData optimization for same-files (#1579)
X-Git-Tag: 2.16.24+jp-2~5^2~6
X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=c9c20e85647ca6d6bb3e388b70b875e3dfb56ed8;p=winmerge-jp%2Fwinmerge-jp.git

DiffFileData optimization for same-files (#1579)

* light optimization

- diffFileData same-files optimization;
- light code generation improvement;

* Add files via upload

* Delete CompareStats.cpp

* Add files via upload
---

diff --git a/Src/CompareStats.cpp b/Src/CompareStats.cpp
index 5875eab91..689d4f040 100644
--- a/Src/CompareStats.cpp
+++ b/Src/CompareStats.cpp
@@ -39,7 +39,7 @@ void CompareStats::AddItem(int code)
 	{
 		RESULT res = GetResultFromCode(code);
 		int index = static_cast<int>(res);
-		m_counts[index] += 1;
+		m_counts[index]++;
 	}
 	++m_nComparedItems;
 	assert(m_nComparedItems <= m_nTotalItems);
@@ -108,21 +108,22 @@ CompareStats::RESULT CompareStats::GetResultFromCode(unsigned diffcode) const
 {
 	DIFFCODE di(diffcode);
 	
+	bool is_dir = di.isDirectory();
 	// Test first for skipped so we pick all skipped items as such 
 	if (di.isResultFiltered())
 	{
 		// skipped
-		return di.isDirectory() ? RESULT_DIRSKIP : RESULT_SKIP;
+		return is_dir ? RESULT_DIRSKIP : RESULT_SKIP;
 	}
 	else if (di.isSideFirstOnly())
 	{
 		// left-only
-		return di.isDirectory() ? RESULT_LDIRUNIQUE : RESULT_LUNIQUE;
+		return is_dir ? RESULT_LDIRUNIQUE : RESULT_LUNIQUE;
 	}
 	else if (di.isSideSecondOnly())
 	{
 		// right-only
-		if (di.isDirectory())
+		if (is_dir)
 			return (m_nDirs < 3) ? RESULT_RDIRUNIQUE : RESULT_MDIRUNIQUE;
 		else
 			return (m_nDirs < 3) ? RESULT_RUNIQUE : RESULT_MUNIQUE;
@@ -130,15 +131,18 @@ CompareStats::RESULT CompareStats::GetResultFromCode(unsigned diffcode) const
 	else if (di.isSideThirdOnly())
 	{
 		// right-only
-		return di.isDirectory() ? RESULT_RDIRUNIQUE : RESULT_RUNIQUE;
+		return is_dir ? RESULT_RDIRUNIQUE : RESULT_RUNIQUE;
 	}
-	else if (m_nDirs > 2 && !di.exists(0) && di.exists(1) && di.exists(2))
-		return di.isDirectory() ? RESULT_LDIRMISSING : RESULT_LMISSING;
-	else if (m_nDirs > 2 && di.exists(0) && !di.exists(1) && di.exists(2))
-		return di.isDirectory() ? RESULT_MDIRMISSING : RESULT_MMISSING;
-	else if (m_nDirs > 2 && di.exists(0) && di.exists(1) && !di.exists(2))
-		return di.isDirectory() ? RESULT_RDIRMISSING : RESULT_RMISSING;
-	else if (di.isResultError())
+	else if (m_nDirs > 2)
+	{
+		switch (diffcode & DIFFCODE::ALL)
+		{
+		case (DIFFCODE::SECOND | DIFFCODE::THIRD) : return is_dir ? RESULT_LDIRMISSING : RESULT_LMISSING;
+		case (DIFFCODE::FIRST  | DIFFCODE::THIRD) : return is_dir ? RESULT_MDIRMISSING : RESULT_MMISSING;
+		case (DIFFCODE::FIRST  | DIFFCODE::SECOND): return is_dir ? RESULT_RDIRMISSING : RESULT_RMISSING;
+		}
+	}
+	if (di.isResultError())
 	{
 		// could be directory error ?
 		return RESULT_ERROR;
@@ -147,7 +151,7 @@ CompareStats::RESULT CompareStats::GetResultFromCode(unsigned diffcode) const
 	else if (di.isResultSame())
 	{
 		// same
-		if (di.isDirectory())
+		if (is_dir)
 		{
 			return RESULT_DIRSAME;
 		}
@@ -158,7 +162,7 @@ CompareStats::RESULT CompareStats::GetResultFromCode(unsigned diffcode) const
 	}
 	else
 	{
-		if (di.isDirectory())
+		if (is_dir)
 		{
 			return RESULT_DIRDIFF;
 		}
diff --git a/Src/DiffFileData.cpp b/Src/DiffFileData.cpp
index d211fad9e..c3711f675 100644
--- a/Src/DiffFileData.cpp
+++ b/Src/DiffFileData.cpp
@@ -26,7 +26,7 @@ DiffFileData::DiffFileData()
 : m_inf(new file_data[2]{})
 , m_used(false)
 {
-	Reset();
+	//Reset(); //this call not needed because memset implicitly used in line 26
 }
 
 /** @brief deallocate member data */
@@ -54,12 +54,13 @@ void DiffFileData::SetDisplayFilepaths(const String& szTrueFilepath1, const Stri
 	m_sDisplayFilepath[1] = szTrueFilepath2;
 }
 
-
 /** @brief Open file descriptors in the inf structure (return false if failure) */
 bool DiffFileData::DoOpenFiles()
 {
 	Reset();
 
+	const bool same_file = !strutils::compare_nocase(m_FileLocation[0].filepath, m_FileLocation[1].filepath);
+
 	for (int i = 0; i < 2; ++i)
 	{
 		// Fill in 8-bit versions of names for diffutils (WinMerge doesn't use these)
@@ -77,20 +78,19 @@ bool DiffFileData::DoOpenFiles()
 		{
 			_tsopen_s(&m_inf[i].desc, TFile(m_FileLocation[i].filepath).wpath().c_str(),
 					O_RDONLY | O_BINARY, _SH_DENYNO, _S_IREAD);
-		}
-		if (m_inf[i].desc < 0)
-			return false;
 
-		// Get file stats (diffutils uses these)
-		if (myfstat(m_inf[i].desc, &m_inf[i].stat) != 0)
-		{
-			return false;
+			if (m_inf[i].desc < 0)
+				return false;
+
+			// Get file stats (diffutils uses these)
+			if (myfstat(m_inf[i].desc, &m_inf[i].stat) != 0)
+				return false;
 		}
 		
-		if (strutils::compare_nocase(m_FileLocation[0].filepath,
-				m_FileLocation[1].filepath) == 0)
+		if (same_file)
 		{
 			m_inf[1].desc = m_inf[0].desc;
+			m_inf[1].stat = m_inf[0].stat;
 		}
 	}
 
@@ -110,20 +110,18 @@ void DiffFileData::Reset()
 	}
 	// clean up any open file handles, and zero stuff out
 	// open file handles might be leftover from a failure in DiffFileData::OpenFiles
+	if (m_inf[1].desc == m_inf[0].desc)
+	{
+		m_inf[1].desc = 0;
+	}
 	for (int i = 0; i < 2; ++i)
 	{
-		if (m_inf[1].desc == m_inf[0].desc)
-		{
-			m_inf[1].desc = 0;
-		}
 		free((void *)m_inf[i].name);
-		m_inf[i].name = nullptr;
 
 		if (m_inf[i].desc > 0)
 		{
 			_close(m_inf[i].desc);
 		}
-		m_inf[i].desc = 0;
 		m_inf[i] = {};
 	}
 }
diff --git a/Src/DiffItem.h b/Src/DiffItem.h
index 4c581ffa6..72e7f9d25 100644
--- a/Src/DiffItem.h
+++ b/Src/DiffItem.h
@@ -133,10 +133,7 @@ public:
 	}
 	bool existAll() const
 	{
-		if ((diffcode & DIFFCODE::THREEWAY) == 0)
-			return (existsFirst() && existsSecond());
-		else
-			return (existsFirst() && existsSecond() && existsThird());
+		return ((diffcode & DIFFCODE::THREEWAY) ? DIFFCODE::ALL : DIFFCODE::BOTH) == (diffcode & DIFFCODE::ALL);
 	}
 
 	// compare result