ファイル読み込み高速化(行データ読み出しのマルチスレッド対応) #2021

suconbu · 2025-03-15T15:32:35Z

PR対象

アプリ(サクラエディタ本体)

カテゴリ

改善

PR の背景

#1988 の第3弾対応です。

今回の変更対象は CReadManager::ReadFile_To_CDocLineMgr() で、この関数では主に以下の処理をしています。

ファイルからテキストデータを読み込む
それを行単位に分割
それを内部文字エンコーディング (UTF-16) へ変換 (CIoBridge::FileToImpl の実行)

これらの処理を yoshinrt/SakuraVz の 1578dc68, e9cbf1b0 の対応を参考に高速化します。

仕様・動作説明

対応内容：

CFileLoad のメモリマップドファイル対応
従来は固定長のバッファに対して逐次ファイル内容を読み込みながら処理をしていましたが、
メモリコピーの負荷を減らすことと、後述のマルチスレッド対応の効果を高める (＝マルチスレッド化ができない処理を減らす) ことを目的に、メモリマップドファイルを使ってデータを読み込むよう変更します。
CReadManager::ReadFile_To_CDocLineMgr のマルチスレッド対応
テキストデータを行単位へ分割する処理と文字コード変換の処理を、複数のスレッドで並列実行するようにします。
サイズが2GiB以上のファイルを開いた時に落ちる問題を修正 (参考元の対応にはない修正)
「2」の対応したことで起きるようになったため本PRに含めて対応します。
CEol::SetTypeByStringForFile() の引数 nDataLen がオーバーフローしていることが分かったため、型を int から size_t に変更します。(関連する変数/引数の型もあわせて直します)

各区間の処理時間 (計測条件などは #1988 を参照)：

区間	対応前 (`9f9948d`)	本PR対応後
A. CLoadAgent::OnLoad 開始から ReadFile_To_CDocLineMgr 完了まで	1458.327	502.304
B. そこから CLoadAgent::OnLoad 完了まで (レイアウトデータ化処理)	676.073	647.408
C. CLoadAgent::OnAfterLoad 開始から完了まで	16.088	89.502
合計	2150.488	1239.214 (-42%)

PR の影響範囲

メモリマップドファイルを使うようにした影響で、ファイル読み込み中の瞬間最大のメモリ使用量が増えます。
読み込んだファイルとほぼ同じサイズ分だけ増加します。

(例) 1GiB のファイルを読み込む時の最大メモリ使用量：

変更前 (`9f9948d`)	本PR対応後
2.4GB	3.3GB (+0.9GB)

テスト内容

test.zip の「test_1000lines.txt」(1000行のテキストファイル) を開き、各行の数字が行番号と一致していること。👈ファイル読み込み処理後のCDocLineMgrの集約が正しい順序でできていることの確認
test_4gb.zip の「test_4gb.txt」(4GBのテキストファイル) を開いた時、エラーのダイアログボックスが表示されないこと。👈対応内容の「3」の修正が効いていることの確認

参考資料

SetTypeByStringForFileの引数nDataLenがオーバーフローしていたため。関連する処理を含めてintをsize_tに変更する。

suconbu · 2025-03-15T15:41:12Z

sakura_core/io/CFileLoad.h

@@ -105,11 +103,14 @@ class CFileLoad
 	/* メンバオブジェクト */
 	const SEncodingConfig* m_pEencoding;

+	//! 文字コード自動検出のために読み込む最大サイズ(byte)
+	static constexpr LONGLONG m_nAutoDetectReadLen = 32768LL;


挙動が変化しないよう旧 gm_nBufSizeDef と同値としました。

suconbu · 2025-03-15T15:43:11Z

sakura_core/io/CFileLoad.cpp

-	バッファにデータを読み込む
-	@note エラー時は throw する
-*/
-void CFileLoad::Buffering( void )


メモリマップドファイル化により逐次の読み込みが不要になったため廃止

AppVeyorBot · 2025-03-15T16:17:18Z

✅ Build sakura 1.0.4461 completed (commit 0a57687cf0 by @suconbu)

berryzplus

対応ありがとうございます。基本方針賛同です。

berryzplus · 2025-03-16T03:38:59Z

sakura_core/CEol.cpp

+	{ "\x0d\x00",			"\x00\x0d",				2U },
+	{ "\x85\x00",			"\x00\x85",				2U },
+	{ "\x28\x20",			"\x20\x28",				2U },
+	{ "\x29\x20",			"\x20\x29",				2U },


指摘ではないですが、変更する必要の「ある／なし」で言うと「なし」なので微妙です。

エンディアン変換をするための構造体だと思いますが、もう少しスマートな方法でやれないものか、とか思います。

int -> size_t (符号なし) に変更したので整数リテラルの符号有無を合わせてますが、変えなくてもwarningは出ないようなのでそのままでも良さそうです。

berryzplus · 2025-03-16T03:47:42Z

sakura_core/CEol.h

-	bool StartsWith(const WCHAR* pData, int nLen) const{ return m_nLen<=nLen && 0==wmemcmp(pData,m_szDataW,m_nLen); }
-	bool StartsWith(const ACHAR* pData, int nLen) const{ return m_nLen<=nLen && m_szDataA[0] != '\0' && 0==memcmp(pData,m_szDataA,m_nLen); }
+	bool StartsWith(const WCHAR* pData, size_t nLen) const{ return m_nLen<=nLen && 0==wmemcmp(pData,m_szDataW,m_nLen); }
+	bool StartsWith(const ACHAR* pData, size_t nLen) const{ return m_nLen<=nLen && m_szDataA[0] != '\0' && 0==memcmp(pData,m_szDataA,m_nLen); }


指摘ではないですが、 util/string_ex.h に似たような定義がたくさんあるので、そちらで集中管理してもいいんじゃないかと思いました。

berryzplus · 2025-03-16T03:49:13Z

sakura_core/CReadManager.cpp

@@ -33,6 +33,8 @@
 #include "util/window.h"
 #include "CSelectLang.h"
 #include "String_define.h"
+#include <atomic>
+#include <future>


指摘ではないですが、標準c++のヘッダーファイルは StdAfx.h でまとめてインクルードする感じにできませんかね。

たぶん趣味の領域なのでどちらでもいいです。

berryzplus · 2025-03-16T03:51:00Z

sakura_core/CReadManager.cpp

@@ -84,7 +86,7 @@ EConvertResult CReadManager::ReadFile_To_CDocLineMgr(
 	EConvertResult eRet = RESULT_COMPLETE;

 	try{
-		CFileLoad cfl(type->m_encoding);
+		CFileLoad cFileLoad( type->m_encoding );


何故変数名を変える？　のありがち指摘を入れときます。

理由があるなら教えてください。
理由のない変更は（以下省略。）

戻しマス

berryzplus · 2025-03-16T03:59:34Z

sakura_core/CReadManager.cpp

 		pFileInfo->SetBomExist( bBom );

 		/* ファイル時刻の取得 */
 		FILETIME	FileTime;
-		if( cfl.GetFileTime( NULL, NULL, &FileTime ) ){
+		if( cFileLoad.GetFileTime( NULL, NULL, &FileTime ) ){


影響が大きいので黙ってましたが、 NULL を nullptr で re-define する変更を revert したいと思っています。
理由は「 gdiplus.h をインクルードしたときエラーになるから」です。
個人的には gdiplus 嫌いなんですけど、ダークモード対応で TortoiseGit のコードの一部を流用しようとしたときに問題になるので、いつか対応入れると思います。

このあたり踏まえて、触る行に含まれる NULL は nullptr に置換しときたいです。

関数内で混在になると気持ち悪いので今回は NULL のままで行かせてもらおうと思います。

忘れてしまいそうなのでコーディング時のガイドライン(？)的なものをWikiにこしらえるのがよさそうです。
(標準ヘッダは StdAfx.h でまとめてインクルード、の件等と合わせて)

berryzplus · 2025-03-16T04:03:20Z

sakura_core/CReadManager.cpp

+		std::vector<std::future<EConvertResult>> vecWorkerFutures;
+		std::atomic<bool> bCanceled = false;
+
+		size_t nOffsetBegin = cFileLoad.GetNextLineOffset( (size_t)cFileLoad.GetFileSize() );


指摘じゃないですが (size_t) に違和感。
キャストが必要なのは縮小変換する場合だけのはず。
cFileLoad.GetFileSize() の関数名で、返すものがファイルサイズじゃないとすると問題。

符号有無が変化することと、32bitビルド時には縮小変換になるため明示的にキャストしてます。
(LONGLONG：8バイト符号あり、size_t：8/4バイト符号なし)

※32bitビルド時は縮小変換になってしまいますが、CFileLoad::IsLoadableSize の判定にて事前にファイルサイズが2GiB未満に制限されているのでオーバーフローの危険なしと判断

理屈は分かりました。

～ここから別な話～
・32bitビルドでも問題ないコードであること、を重要視したくないです。
　理由： 32bit Windows は 2025年10月14日にサポート終了となるため。
・話からするとファイルサイズと「次行の開始オフセット」は ULONGLONG とするべきです。
　他のエディタと合わせたり、システムサービスの恩恵を受けるために、一度に読み込むテキストの総量は LONG に収まるサイズとすべきですが、「全体サイズが 2GB を超えない」は実装の都合なので、その気になれば制約を回避できる感じにしといたほうがいい気がします。

他のエディタと合わせたり、システムサービスの恩恵を受けるために、一度に読み込むテキストの総量は LONG に収まるサイズとすべきですが、「全体サイズが 2GB を超えない」は実装の都合なので、その気になれば制約を回避できる感じにしといたほうがいい気がします。

64bitビルドについて言えば、2GBの制限はされず※、また size_t は8バイト符号なし (ULONGLONG と同じ) となるので上記については問題ないと思います。
※

sakura/sakura_core/io/CFileLoad.cpp

Lines 65 to 76 in a8bf87b

ULONGLONG CFileLoad::GetLimitSize()

{

#ifdef _WIN64

// 64bit の場合

// 実質上限は設けないこととする (x64 対応しながらここは要検討)

return ULLONG_MAX;

#else

// 32bit の場合

// だいたい 2GB くらいを上限とする (既存コードがそうなっていたのでそれを踏襲)

return 0x80000000;

#endif

}

berryzplus · 2025-03-16T04:07:09Z

sakura_core/doc/logic/CDocLineMgr.cpp

+//! 元のCDocLineMgrの内容は空になる
+void CDocLineMgr::AppendAsMove( CDocLineMgr& other )
+{
+	if( other.GetDocLineTop() == nullptr ){ return; }


指摘じゃないです。ほほ趣味ですが、こう書いても同じ意味。

Suggested change

if( other.GetDocLineTop() == nullptr ){ return; }

if (!other.GetDocLineTop()) return; // otherが空なら抜ける

GetDocLineTop() がもしbool型なら GetDocLineTop() == false とせず !GetDocLineTop() とするところですが、ポインタ型なので比較演算子にしました。

どっちでもいいみたいです。

誤って代入するのを防止する目的で、
定数を左辺に書く
みたいな文化もあります。

if ( nullptr == xxx() ){

berryzplus · 2025-03-16T04:08:52Z

sakura_core/io/CFileLoad.h

 //	LPWSTR	m_pszFileName;	// ファイル名
 	HANDLE	m_hFile;		// ファイルハンドル
+	HANDLE	m_hFileMapping;	// メモリマップドファイルハンドル


Suggested change

HANDLE m_hFileMapping; // メモリマップドファイルハンドル

HANDLE m_hFileMapping = nullptr; // メモリマップドファイルハンドル

berryzplus · 2025-03-16T04:12:39Z

sakura_core/io/CFileLoad.h

+	const char*	m_pReadBufTop;	// 読み込みバッファの先頭を指すポインタ
+	size_t m_nReadBufOffsetBegin;
+	size_t m_nReadBufOffsetEnd;
+	size_t m_nReadBufOffsetCurrent;	// 読み込みバッファ中のオフセット(次の行頭位置)


指摘です。最近のC++ではクラスメンバーの宣言時に初期値を書いておくやり方が主流なので、想定する初期値があればここに書いて欲しいです。

宣言時に初期値を入れる方法にすべきことに同意ですが、変更前および既存の他のメンバ変数の初期化方法に倣いました。

宣言時に初期値を入れるよう直すのは今回変更分だけでよいですか？

宣言時に初期値を入れるよう直すのは今回変更分だけでよいですか？

変更箇所についての指摘です。
あくまで「今回増やしたメンバーに（設計値があれば）宣言時に書いて欲しい」です。

変更箇所以外を触ると「なんで触った～～～」になります。

berryzplus · 2025-03-16T04:14:11Z

sakura_core/io/CFileLoad.h

@@ -154,7 +154,7 @@ inline BOOL CFileLoad::GetFileTime( FILETIME* pftCreate, FILETIME* pftLastAccess
 inline int CFileLoad::Read( void* pBuf, size_t nSize )
 {
 	DWORD ReadSize;
-	if( !::ReadFile( m_hFile, pBuf, nSize, &ReadSize, NULL ) )
+	if( !::ReadFile( m_hFile, pBuf, (DWORD)nSize, &ReadSize, NULL ) )


指摘じゃないです。

メモリマップトファイルに変更したのに ReadFile 使うん？
（ちゃんとソースコード読めよ、のセルフ突っ込みしておきます。）

(今回の変更で削除した) CFileLoad::Buffering が唯一の呼び元でした。
下にある CFileLoad::FilePointer については今回の変更前から呼び元おらず。

2つまとめて削除しておきます。

sonarqubecloud · 2025-03-23T16:17:39Z

Quality Gate passed

Issues
13 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

AppVeyorBot · 2025-03-23T16:43:28Z

✅ Build sakura 1.0.4470 completed (commit 9599834505 by @suconbu)

suconbu · 2025-04-19T05:52:30Z

指摘箇所については修正できていると思いますので、問題なさそうであればApproveよろしくお願いします。

Copilot

Pull Request Overview

This PR speeds up file loading by switching to memory‐mapped files and introducing multi-threaded line reading while also addressing a 2GiB file overflow bug. Key changes include updating CFileLoad for memory mapping and offset handling, parallelizing line processing in CReadManager, and updating types for improved correctness in various modules such as CEol and CDocLineMgr.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

File	Description
sakura_core/io/CFileLoad.h, CFileLoad.cpp	Add memory-mapped file support and update offset & buffer handling for improved performance and 2GiB bug fix
sakura_core/doc/logic/CDocLineMgr.*	Change memory resource ownership to shared_ptr and add AppendAsMove for efficient line aggregation
sakura_core/CReadManager.h, CReadManager.cpp	Implement multi-threaded file reading using std::async and integrate line merging from multiple threads
sakura_core/CEol.h, CEol.cpp	Update type signatures from int to size_t for length-related parameters to ensure correctness

Comments suppressed due to low confidence (1)

sakura_core/CReadManager.cpp:120

[nitpick] Consider renaming 'nOffsetBegin' and 'nOffsetEnd' to more descriptive names such as 'chunkStartOffset' and 'chunkEndOffset' to make their roles in file partitioning for multi-threaded processing clearer.

const size_t nOffsetEnd = nOffsetBegin;

Copilot · 2025-05-05T03:19:29Z

sakura_core/CReadManager.cpp

+		size_t nOffsetBegin = cfl.GetNextLineOffset( (size_t)cfl.GetFileSize() );
+		for( int i = nThreadCount - 1; 0 <= i; i-- ){
+			// 分担する範囲を決める
+			const size_t nOffsetEnd = nOffsetBegin;
+			nOffsetBegin = cfl.GetNextLineOffset( (size_t)((double)cfl.GetFileSize() / nThreadCount * i) );


[nitpick] Refactor the file partitioning calculation by using intermediate variables or inline comments to clarify the intent and improve readability.

Suggested change

size_t nOffsetBegin = cfl.GetNextLineOffset( (size_t)cfl.GetFileSize() );

for( int i = nThreadCount - 1; 0 <= i; i-- ){

// 分担する範囲を決める

const size_t nOffsetEnd = nOffsetBegin;

nOffsetBegin = cfl.GetNextLineOffset( (size_t)((double)cfl.GetFileSize() / nThreadCount * i) );

// Calculate the total file size and initialize the starting offset

const size_t nFileSize = (size_t)cfl.GetFileSize();

size_t nOffsetBegin = cfl.GetNextLineOffset(nFileSize);

for( int i = nThreadCount - 1; 0 <= i; i-- ){

// Determine the range of file offsets to be processed by this thread

const size_t nOffsetEnd = nOffsetBegin;

const double partitionSize = (double)nFileSize / nThreadCount; // Size of each partition

const size_t partitionStartOffset = (size_t)(partitionSize * i); // Start offset for this partition

nOffsetBegin = cfl.GetNextLineOffset(partitionStartOffset);

いや、そこまでやらんでもいいと思いました。

suconbu · 2025-05-10T14:05:39Z

ありがとうございます。マージします。

suconbu added 3 commits March 15, 2025 17:36

ファイル読み込み高速化(CFileLoadのメモリマップドファイル対応)

5ff14e2

ファイル読み込み高速化(行データ読み出しのマルチスレッド対応)

289a4bd

2GiB以上のファイルを開く時にエラーが出ることがあるのを修正

a8bf87b

SetTypeByStringForFileの引数nDataLenがオーバーフローしていたため。関連する処理を含めてintをsize_tに変更する。

suconbu commented Mar 15, 2025

View reviewed changes

suconbu changed the title ~~Feature/speedup fileopen multithread open~~ ファイル読み込み高速化(行データ読み出しのマルチスレッド対応) Mar 15, 2025

berryzplus reviewed Mar 16, 2025

View reviewed changes

berryzplus mentioned this pull request Mar 16, 2025

半角文字が全角文字扱いされる + 全角文字が半角文字扱いされる #2022

Closed

レビュー指摘等対応

8536ef0

beru added the 🚅 speed up 🚀 高速化 label Apr 13, 2025

berryzplus requested a review from Copilot May 5, 2025 03:18

Copilot AI reviewed May 5, 2025

View reviewed changes

berryzplus approved these changes May 5, 2025

View reviewed changes

suconbu merged commit 3faa6ae into sakura-editor:master May 10, 2025
25 checks passed

suconbu mentioned this pull request May 10, 2025

巨大なファイルを開く時の待ち時間を短くしたい #1988

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ファイル読み込み高速化(行データ読み出しのマルチスレッド対応) #2021

ファイル読み込み高速化(行データ読み出しのマルチスレッド対応) #2021

suconbu commented Mar 15, 2025 •

edited

Loading

suconbu Mar 15, 2025

suconbu Mar 15, 2025

AppVeyorBot commented Mar 15, 2025

berryzplus left a comment

berryzplus Mar 16, 2025

suconbu Mar 22, 2025

berryzplus Mar 16, 2025

berryzplus Mar 16, 2025

berryzplus Mar 16, 2025

suconbu Mar 22, 2025

berryzplus Mar 16, 2025

suconbu Mar 22, 2025

berryzplus Mar 16, 2025

suconbu Mar 22, 2025

berryzplus Mar 23, 2025

suconbu Mar 23, 2025

berryzplus Mar 16, 2025

suconbu Mar 22, 2025

berryzplus Mar 23, 2025

berryzplus Mar 16, 2025

berryzplus Mar 16, 2025

suconbu Mar 22, 2025

berryzplus Mar 23, 2025

berryzplus Mar 16, 2025

suconbu Mar 22, 2025

sonarqubecloud bot commented Mar 23, 2025

AppVeyorBot commented Mar 23, 2025

suconbu commented Apr 19, 2025

Copilot AI left a comment

Copilot AI May 5, 2025

berryzplus May 5, 2025

suconbu commented May 10, 2025

	ULONGLONG CFileLoad::GetLimitSize()
	{
	#ifdef _WIN64
	// 64bit の場合
	// 実質上限は設けないこととする (x64 対応しながらここは要検討)
	return ULLONG_MAX;
	#else
	// 32bit の場合
	// だいたい 2GB くらいを上限とする (既存コードがそうなっていたのでそれを踏襲)
	return 0x80000000;
	#endif
	}

	if( other.GetDocLineTop() == nullptr ){ return; }
	if (!other.GetDocLineTop()) return; // otherが空なら抜ける

	HANDLE m_hFileMapping; // メモリマップドファイルハンドル
	HANDLE m_hFileMapping = nullptr; // メモリマップドファイルハンドル

ファイル読み込み高速化(行データ読み出しのマルチスレッド対応) #2021

ファイル読み込み高速化(行データ読み出しのマルチスレッド対応) #2021

Conversation

suconbu commented Mar 15, 2025 • edited Loading

PR対象

カテゴリ

PR の背景

仕様・動作説明

PR の影響範囲

テスト内容

関連 issue, PR

参考資料

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AppVeyorBot commented Mar 15, 2025

berryzplus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 23, 2025

Quality Gate passed

AppVeyorBot commented Mar 23, 2025

suconbu commented Apr 19, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Copilot AI May 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suconbu commented May 10, 2025

suconbu commented Mar 15, 2025 •

edited

Loading