Write the detect cycles function as a function to optimize code

[wdfk-prog]

• Using the master branch code, the footprint size of the gcc compilation under Windows is as follows

     text    data     bss     dec     hex filename
  32828       0       0   32828    803c lfs.o
    232       0       0     232      e8 lfs_util.o
  33060       0       0   33060    8124 (TOTALS)

• After this change, the compilation footprint is as follows

   text    data     bss     dec     hex filename
  32680      16       0   32696    7fb8 lfs.o
    232       0       0     232      e8 lfs_util.o
  32912      16       0   32928    80a0 (TOTALS)

• The optimization result can be obtained,Compiles without error or warning.

  33060- 32928 = 132
  148/33060 = 0.4%

• gcc version

Using built-in specs.
COLLECT_GCC=C:\MinGW\bin\gcc.exe
COLLECT_LTO_WRAPPER=c:/mingw/bin/../libexec/gcc/mingw32/9.2.0/lto-wrapper.exe
Target: mingw32
Configured with: ../src/gcc-9.2.0/configure --build=x86_64-pc-linux-gnu --host=mingw32 --target=mingw32 --disable-win32-registry --with-arch=i586 --with-tune=generic --enable-static --enable-shared --enable-threads --enable-languages=c,c++,objc,obj-c++,fortran,ada --with-dwarf2 --disable-sjlj-exceptions --enable-version-specific-runtime-libs --enable-libgomp --disable-libvtv --with-libiconv-prefix=/mingw --with-libintl-prefix=/mingw --enable-libstdcxx-debug --disable-build-format-warnings --prefix=/mingw --with-gmp=/mingw --with-mpfr=/mingw --with-mpc=/mingw --with-isl=/mingw --enable-nls --with-pkgversion='MinGW.org GCC Build-2'
Thread model: win32
gcc version 9.2.0 (MinGW.org GCC Build-2)

[wdfk-prog]

Also worth mentioning is the use of gcc and makefiles on Windows; Can only compile and clear, view the overall occupancy size and generate assembly files, other operations can not take effect, will give an error; Is there any plan to support this part on Windows.

[wdfk-prog]

• CI errors are all Error: The operation was canceled.

[BenBE]

The minor style stuff aside, I think there's a slight problem with that algorithm as it changes behaviour by moving the initialization part into the loop. This might still find trivial cases of loops, but will hide loops, that need the iteration behaviour of advancing the tortoises. You can check this by noticing that several of the assignments of the function lfs_detect_cycles are now dead stores, when previously they would be re-used in the next loop iteration. Nonetheless, centralizing this check pattern may allow code size reduction.

To fix that bug, the following should be done:

1. Introduce a structure to hold the state of the algorithm

typedef struct lfs_tortoise {
    lfs_block_t block[2]; // Previously tortoise
    lfs_size_t i; // Previously tortoise_i
    lfs_size_t period; // Previously tortoise_period
} lfs_tortoise_t;

2. Pass a lfs_tortoise_t* as the second argument to lfs_detect_cycles

3. On the call sites: Outside the loop initialize a lfs_tortoise_t like so:

lfs_tortoise_t tortoise = {
    .block = {LFS_BLOCK_NULL, LFS_BLOCK_NULL},
    .i = 1,
    .period = 1,
};

4. Pass that variable to lfs_detect_cycles by reference.

The "cancelled" CI jobs were likely due to some loop in the filesystem under test not being detected after your change.

[wdfk-prog]

The minor style stuff aside, I think there's a slight problem with that algorithm as it changes behaviour by moving the initialization part into the loop. This might still find trivial cases of loops, but will hide loops, that need the iteration behaviour of advancing the tortoises. You can check this by noticing that several of the assignments of the function lfs_detect_cycles are now dead stores, when previously they would be re-used in the next loop iteration. Nonetheless, centralizing this check pattern may allow code size reduction.撇开次要风格的东西不谈，我认为该算法存在一个小问题，因为它通过将初始化部分移动到循环中来改变行为。这仍然可能找到循环的微不足道的情况，但会隐藏循环，这些循环需要推进的迭代行为。您可以通过注意到函数lfs_detect_cycles的几个赋值现在是死存储来检查这一点，而以前它们将在下一次循环迭代中重用。尽管如此，集中管理此检查模式可能会减小代码大小。

To fix that bug, the following should be done:要修复该错误，应执行以下操作：

1. Introduce a structure to hold the state of the algorithm引入一个结构来保存算法的状态

typedef struct lfs_tortoise {
    lfs_block_t block[2]; // Previously tortoise
    lfs_size_t i; // Previously tortoise_i
    lfs_size_t period; // Previously tortoise_period
} lfs_tortoise_t;

2. Pass a lfs_tortoise_t* as the second argument to 将 lfs_tortoise_t* 作为第二个参数传递给lfs_detect_cycles
3. On the call sites: Outside the loop initialize a lfs_tortoise_t like so:在调用站点上：在循环外初始化一个lfs_tortoise_t，如下所示：

lfs_tortoise_t tortoise = {
    .block = {LFS_BLOCK_NULL, LFS_BLOCK_NULL},
    .i = 1,
    .period = 1,
};

4. Pass that variable to lfs_detect_cycles by reference.通过引用将该变量传递给 lfs_detect_cycles。

The "cancelled" CI jobs were likely due to some loop in the filesystem under test not being detected after your change.“已取消”的 CI 作业可能是由于在更改后未检测到所测试文件系统中的某些循环。

The minor style stuff aside, I think there's a slight problem with that algorithm as it changes behaviour by moving the initialization part into the loop. This might still find trivial cases of loops, but will hide loops, that need the iteration behaviour of advancing the tortoises. You can check this by noticing that several of the assignments of the function lfs_detect_cycles are now dead stores, when previously they would be re-used in the next loop iteration. Nonetheless, centralizing this check pattern may allow code size reduction.

To fix that bug, the following should be done:

1. Introduce a structure to hold the state of the algorithm

typedef struct lfs_tortoise {
    lfs_block_t block[2]; // Previously tortoise
    lfs_size_t i; // Previously tortoise_i
    lfs_size_t period; // Previously tortoise_period
} lfs_tortoise_t;

2. Pass a lfs_tortoise_t* as the second argument to lfs_detect_cycles
3. On the call sites: Outside the loop initialize a lfs_tortoise_t like so:

lfs_tortoise_t tortoise = {
    .block = {LFS_BLOCK_NULL, LFS_BLOCK_NULL},
    .i = 1,
    .period = 1,
};

4. Pass that variable to lfs_detect_cycles by reference.

The "cancelled" CI jobs were likely due to some loop in the filesystem under test not being detected after your change.

Thank you for reminding me that I made a stupid mistake

[BenBE]

This needs to go outside the loop. Similar for the other 3 places.

[wdfk-prog]

o(╥﹏╥)o,Sorry, I was so careless.

[geky-bot]

Tests passed ✓, Code: 17096 B (+0.2%), Stack: 1448 B (+0.6%), Structs: 812 B (+0.0%)

	Code	Stack	Structs		Coverage
Default	17096 B (+0.2%)	1448 B (+0.6%)	812 B (+0.0%)	Lines	2373/2548 lines (+0.1%)
Readonly	6190 B (-0.1%)	448 B (+0.0%)	812 B (+0.0%)	Branches	1241/1580 branches (-0.1%)
Threadsafe	17956 B (+0.2%)	1448 B (+0.6%)	820 B (+0.0%)		Benchmarks
Multiversion	17164 B (+0.2%)	1448 B (+0.6%)	816 B (+0.0%)	Readed	29369693876 B (+0.0%)
Migrate	18788 B (+0.1%)	1744 B (+0.0%)	816 B (+0.0%)	Proged	1482874766 B (+0.0%)
Error-asserts	17780 B (+0.2%)	1440 B (+0.6%)	812 B (+0.0%)	Erased	1568888832 B (+0.0%)

[geky]

It looks like you're passing the dir by value, instead of by pointer. This creates a copy of the 8-word struct, which might be adding both stack cost and code cost.

Passing by pointer:

static int lfs_detect_cycles(const lfs_mdir_t *dir, struct lfs_tortoise_t *tortoise) {

[geky]

Also worth mentioning is the use of gcc and makefiles on Windows; Can only compile and clear, view the overall occupancy size and generate assembly files, other operations can not take effect, will give an error; Is there any plan to support this part on Windows.

Unfortunately Windows is a difficult platform to support as a development environment. It's just not worth supporting vs leveraging all of the useful tools that are available in Linux.

The best option would be use either a virtual machine or WSL to run a Linux image on windows and develop inside that. I've heard good things about WSL but haven't used it myself.