[ Optimization ]: replace pipe() by pipe2()

[pavlinux]

Linux only :-P

• The pipe2() was added to Linux in version 2.6.27. It is a variant of the normal pipe syscall, but takes an extra flags argument which can be the ORed value of O_NONBLOCK and O_CLOEXEC. The original pipe syscall is now defined as like the pipe2 syscall, but with the flags variable set to zero.

• The pipe2() glibc support is available starting with version 2.9.

• The pipe2() is Linux-specific.

lib/utilunix.c

@@ -340 +340 @@
 void
 open_error_pipe (void)
 {
-    if (pipe (error_pipe) < 0)
+    if (pipe2(error_pipe, O_DIRECT) < 0)
     {
         message (D_NORMAL, _("Warning"), _("Pipe failed"));
     }

src/background.c

@@ -514 +514 @@
     int back_comm[2];           /* back connection */
     pid_t pid;
 
-    if (pipe (comm) == -1)
+    if (pipe2(comm, O_DIRECT) == -1)
         return -1;
 
-    if (pipe (back_comm) == -1)
+    if (pipe2(back_comm, O_DIRECT) == -1)
         return -1;
 
     pid = fork ();

src/cons.handler.c

@@ -148 +148 @@
         status = close (pipefd1[1]);
         status = close (pipefd2[0]);
         /* Create two pipes for communication */
-        if (!((pipe (pipefd1) == 0) && ((pipe (pipefd2)) == 0)))
+        if (!((pipe2(pipefd1, O_DIRECT) == 0) && ((pipe2(pipefd2, O_DIRECT)) == 0)))
         {
             mc_global.tty.console_flag = '\0';
             break;

src/subshell.c

@@ -850 +850 @@
                 return;
             }
         }
-        else /* subshell_type is BASH or ZSH */ if (pipe (subshell_pipe))
+        else /* subshell_type is BASH or ZSH */ if (pipe2(subshell_pipe, O_DIRECT))
         {
             perror (__FILE__ ": couldn't create pipe");
             mc_global.tty.use_subshell = FALSE;

src/vfs/fish/fish.c

@@ -312 +312 @@
     int fileset1[2], fileset2[2];
     int res;
 
-    if ((pipe (fileset1) < 0) || (pipe (fileset2) < 0))
+    if ((pipe2(fileset1, O_DIRECT) < 0) || (pipe2(fileset2, O_DIRECT) < 0))
         vfs_die ("Cannot pipe(): %m.");
 
     res = fork ();

[pavlinux]

P.S.

O_DIRECT flag work only from kernel 3.4 and other backports (for ex. 3.2.19+)

[andrew_b]

Could you make your patch not for Linux only?

[ossi]

regardless of the missing ifdefs, the description also entirely fails to mention what the *point* of this patch is.

[pavlinux]

Replying to andrew_b:

Could you make your patch not for Linux only?

/* define somewhere */

#ifdef linux
     #define mc_pipe(fd)  pipe2(fd, O_DIRECT)
#else 
     #define mc_pipe(fd)  pipe(fd)
#endif

[pavlinux]

]:->

[pavlinux]

Fixed patch.

lib/utilunix.c

@@ -340 +340 @@
 void
 open_error_pipe (void)
 {
-    if (pipe (error_pipe) < 0)
+    if (mc_pipe(error_pipe) < 0)
     {
         message (D_NORMAL, _("Warning"), _("Pipe failed"));
     }

src/background.c

@@ -514 +514 @@
     int back_comm[2];           /* back connection */
     pid_t pid;
 
-    if (pipe (comm) == -1)
+    if (mc_pipe(comm) == -1)
         return -1;
 
-    if (pipe (back_comm) == -1)
+    if (mc_pipe(back_comm) == -1)
         return -1;
 
     pid = fork ();

src/cons.handler.c

@@ -148 +148 @@
         status = close (pipefd1[1]);
         status = close (pipefd2[0]);
         /* Create two pipes for communication */
-        if (!((pipe (pipefd1) == 0) && ((pipe (pipefd2)) == 0)))
+        if (!((mc_pipe(pipefd1) == 0) && ((mc_pipe(pipefd2)) == 0)))
         {
             mc_global.tty.console_flag = '\0';
             break;

src/subshell.c

@@ -850 +850 @@
                 return;
             }
         }
-        else /* subshell_type is BASH or ZSH */ if (pipe (subshell_pipe))
+        else /* subshell_type is BASH or ZSH */ if (mc_pipe(subshell_pipe))
         {
             perror (__FILE__ ": couldn't create pipe");
             mc_global.tty.use_subshell = FALSE;

src/vfs/fish/fish.c

@@ -312 +312 @@
     int fileset1[2], fileset2[2];
     int res;
 
-    if ((pipe (fileset1) < 0) || (pipe (fileset2) < 0))
+    if ((mc_pipe(fileset1) < 0) || (mc_pipe(fileset2) < 0))
         vfs_die ("Cannot pipe(): %m.");
 
     res = fork ();

lib/global.h

@@ -283 +283 @@
     } vfs;
 } mc_global_t;
 
+#if defined(linux) /* compiler definition */
+  #include <linux/version.h> 
+     #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,28)               
+          #define mc_pipe(fd)  pipe2(fd, O_DIRECT)                        
+     #else   
+          #define mc_pipe(fd) pipe(fd)                        
+     #endif
+#else
+     #define mc_pipe(fd) pipe(fd)
+#endif
+
 /*** global variables defined in .c file *********************************************************/
 
 extern mc_global_t mc_global;

[ossi]

you still didn't say how this is supposed to be an optimization. also, some numbers (or at least plausible reasoning) which suggest that complicating the code is even remotely worth it would be in order.

[slyfox]

if ((pipe (fileset1) < 0) + if ((pipe2(fileset1, O_DIRECT) < 0)

(pipe (fileset2) < 0))

(pipe2(fileset2, O_DIRECT) < 0))

Care to explain benefits?

The O_DIRECT addition pessimizes pipe I/O by requiring more read()s:

​https://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=9883035ae7edef3ec62ad215611cb8e17d6a1a5d

[pavlinux]

The O_DIRECT addition pessimizes pipe I/O by requiring more read()s:

...you can now treat the pipe as a packet interface, 
where each read() system call will read one packet at a time...

[pavlinux]

Мужики, кто из нас разработчик миднайта?!
Я предложил фичу, не нравиццо - удаляйте, сомневаетесь - делайте бенчмарки.
На больших потоках данных оно реально даёт прирост, я уже с мая месяца эту фичу
использую в своих проектах.

Google Translate to help you if you do not like the russian :-P

[pavlinux]

Benchmark:

​http://pavlinux.ru/pipe2bench/pipe2bench.c
​http://pavlinux.ru/pipe2bench/Makefile

# make
# ./pipe2bench /media/disk/iso/2011Mb.iso

PIPE Time: 35141 ms
PIPE2 Time: 14922 ms

[ossi]

Replying to pavlinux:

Google Translate to help you if you do not like the russian :-P

yeah, right - as if i'd understand any of that gibberish if i didn't read russian ...
you're just being lazy, which is somewhat consistent with the rest of this contribution. your synthetic benchmark as such doesn't prove anything. for every case where you replaced the pipe() call, you need to identify the expected data flow patterns and make a case that using "packet pipes" improves the performance on average. also, you should make a point that it *makes any practical difference at all* - whether it be higher bandwidth in some particular (realistic) scenario, or just measurably lower cpu usage (and thus power consumption).

[slyfox]

Replying to pavlinux:

Benchmark:

​http://pavlinux.ru/pipe2bench/pipe2bench.c
​http://pavlinux.ru/pipe2bench/Makefile

# make
# ./pipe2bench /media/disk/iso/2011Mb.iso

PIPE Time: 35141 ms
PIPE2 Time: 14922 ms

Benchmarking sync()/fsync() times is very interesting, yes.
Try to explore performance patterns with 'perf' tool'.
It's very simple:

    $ ls -lh /big/dist/os/maemo/n800/RX-34_DIABLO_4.2008.bin
    136MB
    $ perf record ./pipe2bench /big/dist/os/maemo/n800/RX-34_DIABLO_4.2008.bin 
    PIPE  Time: 635 ms
    PIPE2 Time: 336 ms

    perf record ./pipe2bench /big/dist/os/maemo/n800/RX-34_DIABLO_4.2008.bin
    perf report

    29.87%  pipe2bench  [kernel.kallsyms]  [k] copy_user_generic_string
    19.97%  pipe2bench  [kernel.kallsyms]  [k] sync_inodes_sb
     6.61%  pipe2bench  [kernel.kallsyms]  [k] _raw_spin_lock
     3.62%  pipe2bench  [kernel.kallsyms]  [k] find_get_page
     3.06%  pipe2bench  [kernel.kallsyms]  [k] btrfs_drop_inode
     2.76%  pipe2bench  [kernel.kallsyms]  [k] filemap_fdatawait_range
     2.24%  pipe2bench  [kernel.kallsyms]  [k] __srcu_read_lock
     1.67%  pipe2bench  [kernel.kallsyms]  [k] radix_tree_lookup_element
     1.57%  pipe2bench  [kernel.kallsyms]  [k] filemap_fdatawait

Fun, isn't it? You've benched my cache drop speed.
How about second run?

    ./pipe2bench /big/dist/os/maemo/n800/RX-34_DIABLO_4.2008.bin 
    PIPE  Time: 335 ms
    PIPE2 Time: 334 ms

I strongly suggest you to read what pipe's O_DIRECT actually does
and who is intended to be the primary user of it.

You might also like to add a loop to your benchmark to compare a set of
runs or just swap pp2() and pp().

Мужики, кто из нас разработчик миднайта?!
Я предложил фичу, не нравиццо - удаляйте, сомневаетесь - делайте бенчмарки.
На больших потоках данных оно реально даёт прирост, я уже с мая месяца эту фичу
использую в своих проектах.

Это хак для autofs, которая имеет нестабильный ABI user<->kernel
(размер структуры плавает в зависимости от битности ядра ядра):

​http://lwn.net/Articles/494993/

Можешь в бенчмарке побайтно начать писать в pipe и постранично
пытаться считывать - увидишь разницу.

Эта фича не делает pipe() быстрее (можно убедиться на трейсе, выданном perf).

[pavlinux]

2-nd edition :)

​http://pavlinux.ru/pipe2bench/pipe2bench2.c

localhost:/tmp # for ((i=0; i < 5; i++)) do ./pipe2bench2 /media/disk/iso/2011Mb.iso; done;
PIPE  Time: 2283 ms
PIPE2 Time: 1806 ms
PIPE  Time: 2242 ms
PIPE2 Time: 1805 ms
PIPE  Time: 2227 ms
PIPE2 Time: 1795 ms
PIPE  Time: 2215 ms
PIPE2 Time: 1803 ms
PIPE  Time: 2191 ms
PIPE2 Time: 1795 ms

По ассемблерному коду вообще нет различий, так что,
счётчик функций ваааще не показатель!

; pipe() secion
jmp    400b05 <main+0x125>
nopl   0x0(%rax)
mov    0x2015d6(%rip),%edi        # 6020cc <pipefd+0x4>
lea    0x20(%rsp),%rsi
mov    $0x1,%edx
callq  4008d0 <write@plt>
mov    0x2015bd(%rip),%edi        # 6020c8 <pipefd>
lea    0x20(%rsp),%rsi
mov    $0x1000,%edx
callq  400940 <read@plt>
test   %rax,%rax
jg     400af0 <main+0x110>

; pipe2() secion
jmp    400b85 <main+0x1a5>
nopl   0x0(%rax,%rax,1)
mov    0x20154a(%rip),%edi        # 6020c0 <pipe2fd+0x4>
lea    0x20(%rsp),%rsi
mov    $0x1,%edx
callq  4008d0 <write@plt>
mov    0x201531(%rip),%edi        # 6020bc <pipe2fd>
lea    0x20(%rsp),%rsi
mov    $0x1000,%edx
callq  400940 <read@plt>
test   %rax,%rax
jg     400b70 <main+0x190>

[andrew_b]

Rewrite your patch to use m4 to test new function instead of using kernel headers in user-space program.

[slyfox]

Replying to pavlinux:

2-nd edition :)

​http://pavlinux.ru/pipe2bench/pipe2bench2.c

localhost:/tmp # for ((i=0; i < 5; i++)) do ./pipe2bench2 /media/disk/iso/2011Mb.iso; done;
PIPE  Time: 2283 ms
PIPE2 Time: 1806 ms
PIPE  Time: 2242 ms
PIPE2 Time: 1805 ms
PIPE  Time: 2227 ms
PIPE2 Time: 1795 ms
PIPE  Time: 2215 ms
PIPE2 Time: 1803 ms
PIPE  Time: 2191 ms
PIPE2 Time: 1795 ms

По ассемблерному коду вообще нет различий, так что,
счётчик функций ваааще не показатель!

?

perf показывает реальное число инструкций ядра ([k]),
a не функций. Поставь pp2() первым в тесте :]

Что это за железка?

Обычный десктоп на 1G файле:

[sf] /tmp/y:./a /big/mov/film/The\ Art\ of\ Flight.m4v 
PIPE  Time: 10009 ms
PIPE2 Time: 460 ms
[sf] /tmp/y:./a /big/mov/film/The\ Art\ of\ Flight.m4v 
PIPE  Time: 444 ms
PIPE2 Time: 439 ms
[sf] /tmp/y:./a /big/mov/film/The\ Art\ of\ Flight.m4v 
PIPE  Time: 432 ms
PIPE2 Time: 441 ms

[slyfox]

Replying to pavlinux:

2-nd edition :)

​http://pavlinux.ru/pipe2bench/pipe2bench2.c

И подсунь ему файл размером 7 байт и посмотри на трейс:

read(6, "1234567", 4096)                = 7
write(7, "1", 1)                        = 1

hint:

    n = read(pipefd[0], &buf, BUFF_SIZE) > 0

возвращает только 0 или 1.

[pavlinux]

Replying to slyfox:

И подсунь ему файл размером 7 байт и посмотри на трейс:

Без тибя нашёл :-P

3-nd edition :)

​http://pavlinux.ru/pipe2bench/pipe2bench2.c
​http://pavlinux.ru/pipe2bench/Makefile;
​http://pavlinux.ru/pipe2bench/MD5SUM;

83489098d5b6f13c324b5b5b972dcc3d Makefile
ddb4f9e4bbbc8d721a297cc66af247a3 pipe2bench2.c

[pavlinux]

Переставил pp2() и pp() местами

# ./pipe2bench /media/disk/iso/2011Mb.iso 
PIPE  Time: 35948 ms
PIPE2 Time: 12696 ms

В 2 раза!!!

# ./pipe2bench /media/disk/pub/Dist/Windows7/Windows7.Ultimate.32bit.iso 

PIPE  Time: 86984 ms
PIPE2 Time: 21992 ms

Есть разница, 1.5 минуты или 20 сек.?

./pipe2bench /boot/vmlinuz-3.8.0-rc3+ 
PIPE  Time: 159 ms
PIPE2 Time: 113 ms

# ./pipe2bench /media/disk/pub/Distributives/MSVSphere/MSVSphere-Server.rar 
PIPE  Time: 46552 ms
PIPE2 Time: 13746 ms

./pipe2bench /media/disk/pub/Distributives/WindowsXP64/WindowsXP_64_SP2.iso 
PIPE  Time: 9053 ms
PIPE2 Time: 2698 ms

в последнем тесте поменял местами, сначала pipe(), потом pipe2();

Измерять надо при первом обращении к файлу, дальше оно кэшируется.

[gotar]

Replying to andrew_b:

Rewrite your patch to use m4 to test new function instead of using kernel headers in user-space program.

Compile-time detection is not enough, this must be runtime-detectable in order not to break mc when running on pre-2.6.27 kernels as it "will fail with an EINVAL on kernels that do not support this interface".

[slyfox]

Replying to pavlinux:

Replying to slyfox:

И подсунь ему файл размером 7 байт и посмотри на трейс:

Без тибя нашёл :-P

3-nd edition :)

​http://pavlinux.ru/pipe2bench/pipe2bench2.c
​http://pavlinux.ru/pipe2bench/Makefile;
​http://pavlinux.ru/pipe2bench/MD5SUM;

83489098d5b6f13c324b5b5b972dcc3d Makefile
ddb4f9e4bbbc8d721a297cc66af247a3 pipe2bench2.c

Не собирается :p

[slyfox]

Не собирается :p

Хедера sys/stat не хватает для S_ макросов.
В макросах pp() и pp2() фигурные скобки не сбалансированы.
Makefile вообще укуренный и собирает исходник не с тем именем.
target all должен первым идти, чтобы по умолчанию исполняемый
файл собирался.

Лучше на github :]

[pavlinux]

Лучше на github :]

​https://github.com/pavlinux/pipe2bench

[slyfox]

Replying to pavlinux:

Лучше на github :]

​https://github.com/pavlinux/pipe2bench

Чудно. Уже собирается, но всё равно нет различий :]

Измерять надо при первом обращении к файлу, дальше оно кэшируется.

То есть бенчмарк _вообще_ ничего не показывает.
Чтобы сделать его самодостаточным нужно перед каждым "тестом":

• всунуть "sync()"
• всунуть "echo 3 > /proc/sys/vm/drop_caches"

чтобы хотя-бы замерять одно и то же.

А то есть некоторые подозрения :]

[pavlinux]

Replying to slyfox:

Replying to pavlinux:

Лучше на github :]

​https://github.com/pavlinux/pipe2bench

Чудно. Уже собирается, но всё равно нет различий :]

Измерять надо при первом обращении к файлу, дальше оно кэшируется.

То есть бенчмарк _вообще_ ничего не показывает.

Ты в реальной жизни один и тот же файл тоже по 2 раза копируешь?

[slyfox]

Конструктива 0.

Closing as INVALID as benchmark is Obviously Wrong :]