From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#66020: (bug#64735 spin-off): regarding the default for read-process-output-max Date: Thu, 21 Sep 2023 20:33:29 +0300 Message-ID: <1d43341a-2e7c-9f1d-68e9-9bac6f4b5fba@gutov.dev> References: <18a0b4d8-32bd-3ecd-8db4-32608a1ebba7@gutov.dev> <83il8lxjcu.fsf@gnu.org> <2e21ec81-8e4f-4c02-ea15-43bd6da3daa7@gutov.dev> <8334zmtwwi.fsf@gnu.org> <83tts0rkh5.fsf@gnu.org> <831qf3pd1y.fsf@gnu.org> <28a7916e-92d5-77ab-a61e-f85b59ac76b1@gutov.dev> <83sf7jnq0m.fsf@gnu.org> <5c493f86-0af5-256f-41a7-7d886ab4c5e4@gutov.dev> <83ledanvzw.fsf@gnu.org> <83r0n2m7qz.fsf@gnu.org> <26afa109-9ba3-78a3-0e68-7585ae8e3a19@gutov.dev> <83il8dna30.fsf@gnu.org> <83bke5mhvs.fsf@gnu.org> <83a5tmk79p.fsf@gnu.org> <937d9927-506f-aa36-94e9-3cceb8f629dd@gutov.dev> <83zg1hay6q.fsf@gnu.org> <451d6012-e5ab-df6c-50e3-dac20b91781c@gutov.dev> <83led09dlk.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------X2wXDJQTrLHnoyaJptdb0Mbn" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="19362"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Cc: 66020@debbugs.gnu.org, monnier@iro.umontreal.ca, stefankangas@gmail.com To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Sep 21 19:34:24 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qjNZH-0004oQ-34 for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 21 Sep 2023 19:34:23 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qjNYq-0000nV-Dc; Thu, 21 Sep 2023 13:33:56 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qjNYm-0000nN-QJ for bug-gnu-emacs@gnu.org; Thu, 21 Sep 2023 13:33:54 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qjNYm-0006wB-B1 for bug-gnu-emacs@gnu.org; Thu, 21 Sep 2023 13:33:52 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qjNYv-0003bq-KC for bug-gnu-emacs@gnu.org; Thu, 21 Sep 2023 13:34:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 21 Sep 2023 17:34:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 66020 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 66020-submit@debbugs.gnu.org id=B66020.169531763313857 (code B ref 66020); Thu, 21 Sep 2023 17:34:01 +0000 Original-Received: (at 66020) by debbugs.gnu.org; 21 Sep 2023 17:33:53 +0000 Original-Received: from localhost ([127.0.0.1]:34764 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qjNYm-0003bR-GW for submit@debbugs.gnu.org; Thu, 21 Sep 2023 13:33:52 -0400 Original-Received: from out4-smtp.messagingengine.com ([66.111.4.28]:57821) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qjNYk-0003b9-5n for 66020@debbugs.gnu.org; Thu, 21 Sep 2023 13:33:51 -0400 Original-Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 686125C0ABC; Thu, 21 Sep 2023 13:33:33 -0400 (EDT) Original-Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Thu, 21 Sep 2023 13:33:33 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gutov.dev; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1695317613; x=1695404013; bh=DG nJ6hz40L8fhUpX2K1tJ1iJM4MSo4bd/bEkm/5yTfo=; b=2qsGKFrNuJPueZoAL/ lqu0GOu3188LeapqqwbtJJIjBm2F7CdLPe47B4wT3sC5tEpvv212nfZAPm+tbtwG RAVx6d6IDFWlMSyCSUYxQU398tBB972eoWXl/h5fkBh7tS3Rou4C34F6KGPRX9Up ZWgSuJ7juuY7+PnyUPoaxoJZW+G+WbUQiR94qYusfFV1EPHo8p6TRh0AqB2vvpQT 5YIwajPiJ3fqaVUDkQMmTyoEHXqOq6ZPCVPGxBBtBfTK8SrosnVrYuNRyq7fRA8v IsGUJKZ3YsNSb7nJJvigm5gGWw4sZOHzDFRRrHh2QJt0JbqNdaN3saKoRPbwLlOC y7CA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; t=1695317613; x=1695404013; bh=DGnJ6hz40L8fh UpX2K1tJ1iJM4MSo4bd/bEkm/5yTfo=; b=PiCbwzr7t8o9CLwtl6PrpGPnkuXOx Xk5TorRCQRcJXbr3LgVD83ib24Z7d8hS+J3sMe7NOCVwGRLdm/VowEGLEItBq4J4 0ySij+rfUcr0ZlWm7oLvo9TTOU5r6E6SdOfrhaKVSVQYym/P73HexAo4qqwIAhxZ 5Z08AtnsH8gedWkzi2t1FqBIfB7cu/v99do8Dg02Yi3pO4GmF0wJT/2CJY+wMEln MM14IOQrDQaBmB3Zmn2QF83fBjRpMtbzepBhFIptpXK7pSRjwHlFxkWw0jMQq8Xz wbqHKl+IX12MFdmqzzK84QUK06GMNAczll3uBD/pexj+KOYH9G3ZjrjBA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedviedrudekiedguddufecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpegtkfffgggfuffhvfevfhgjsehmtderredtfeejnecuhfhrohhmpeffmhhi thhrhicuifhuthhovhcuoegumhhithhrhiesghhuthhovhdruggvvheqnecuggftrfgrth htvghrnhepieehgeektefggfehhfegieevuedugedvueetfffhtedtleelheekffelvddu tdelnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepug hmihhtrhihsehguhhtohhvrdguvghv X-ME-Proxy: Feedback-ID: i0e71465a:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 21 Sep 2023 13:33:31 -0400 (EDT) Content-Language: en-US In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:271010 Archived-At: This is a multi-part message in MIME format. --------------X2wXDJQTrLHnoyaJptdb0Mbn Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 21/09/2023 17:37, Dmitry Gutov wrote: > We could look into improving that part specifically: for example, > reading from the process multiple times into 'chars' right away while > there is still pending output present (either looping inside > read_process_output, or calling it in a loop in > wait_reading_process_output, at least until the process' buffered output > is exhausted). That could reduce reactivity, however (can we find out > how much is already buffered in advance, and only loop until we exhaust > that length?) Hmm, the naive patch below offers some improvement for the value 4096, but still not comparable to raising the buffer size: 0.76 -> 0.72. diff --git a/src/process.c b/src/process.c index 2376d0f288d..a550e223f78 100644 --- a/src/process.c +++ b/src/process.c @@ -5893,7 +5893,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, && ((fd_callback_info[channel].flags & (KEYBOARD_FD | PROCESS_FD)) == PROCESS_FD)) { - int nread; + int nread = 0, nnread; /* If waiting for this channel, arrange to return as soon as no more input to be processed. No more @@ -5912,7 +5912,13 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, /* Read data from the process, starting with our buffered-ahead character if we have one. */ - nread = read_process_output (proc, channel); + do + { + nnread = read_process_output (proc, channel); + nread += nnread; + } + while (nnread >= 4096); + if ((!wait_proc || wait_proc == XPROCESS (proc)) && got_some_output < nread) got_some_output = nread; And "unlocking" the pipe size on the external process takes the performance further up a notch (by default it's much larger): 0.72 -> 0.65. diff --git a/src/process.c b/src/process.c index 2376d0f288d..85fc1b4d0c8 100644 --- a/src/process.c +++ b/src/process.c @@ -2206,10 +2206,10 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) inchannel = p->open_fd[READ_FROM_SUBPROCESS]; forkout = p->open_fd[SUBPROCESS_STDOUT]; -#if (defined (GNU_LINUX) || defined __ANDROID__) \ - && defined (F_SETPIPE_SZ) - fcntl (inchannel, F_SETPIPE_SZ, read_process_output_max); -#endif /* (GNU_LINUX || __ANDROID__) && F_SETPIPE_SZ */ +/* #if (defined (GNU_LINUX) || defined __ANDROID__) \ */ +/* && defined (F_SETPIPE_SZ) */ +/* fcntl (inchannel, F_SETPIPE_SZ, read_process_output_max); */ +/* #endif /\* (GNU_LINUX || __ANDROID__) && F_SETPIPE_SZ *\/ */ } if (!NILP (p->stderrproc)) Apparently the patch from bug#55737 also made things a little worse by default, by limiting concurrency (the external process has to wait while the pipe is blocked, and by default Linux's pipe is larger). Just commenting it out makes performance a little better as well, though not as much as the two patches together. Note that both changes above are just PoC (e.g. the hardcoded 4096, and probably other details like carryover). I've tried to make a more nuanced loop inside read_process_output instead (as replacement for the first patch above), and so far it performs worse that the baseline. If anyone can see when I'm doing wrong (see attachment), comments are very welcome. --------------X2wXDJQTrLHnoyaJptdb0Mbn Content-Type: text/x-patch; charset=UTF-8; name="read_process_output_nn_inside.diff" Content-Disposition: attachment; filename="read_process_output_nn_inside.diff" Content-Transfer-Encoding: base64 ZGlmZiAtLWdpdCBhL3NyYy9wcm9jZXNzLmMgYi9zcmMvcHJvY2Vzcy5jCmluZGV4IDIzNzZk MGYyODhkLi45MWE1YzA0NGE4YyAxMDA2NDQKLS0tIGEvc3JjL3Byb2Nlc3MuYworKysgYi9z cmMvcHJvY2Vzcy5jCkBAIC02MTI4LDExICs2MTMzLDExIEBAIHJlYWRfYW5kX2Rpc3Bvc2Vf b2ZfcHJvY2Vzc19vdXRwdXQgKHN0cnVjdCBMaXNwX1Byb2Nlc3MgKnAsIGNoYXIgKmNoYXJz LAogc3RhdGljIGludAogcmVhZF9wcm9jZXNzX291dHB1dCAoTGlzcF9PYmplY3QgcHJvYywg aW50IGNoYW5uZWwpCiB7Ci0gIHNzaXplX3QgbmJ5dGVzOworICBzc2l6ZV90IG5ieXRlcywg bm5ieXRlcyA9IDA7CiAgIHN0cnVjdCBMaXNwX1Byb2Nlc3MgKnAgPSBYUFJPQ0VTUyAocHJv Yyk7CiAgIGVhc3NlcnQgKDAgPD0gY2hhbm5lbCAmJiBjaGFubmVsIDwgRkRfU0VUU0laRSk7 CiAgIHN0cnVjdCBjb2Rpbmdfc3lzdGVtICpjb2RpbmcgPSBwcm9jX2RlY29kZV9jb2Rpbmdf c3lzdGVtW2NoYW5uZWxdOwotICBpbnQgY2FycnlvdmVyID0gcC0+ZGVjb2RpbmdfY2Fycnlv dmVyOworICBpbnQgY2FycnlvdmVyOwogICBwdHJkaWZmX3QgcmVhZG1heCA9IGNsaXBfdG9f Ym91bmRzICgxLCByZWFkX3Byb2Nlc3Nfb3V0cHV0X21heCwgUFRSRElGRl9NQVgpOwogICBz cGVjcGRsX3JlZiBjb3VudCA9IFNQRUNQRExfSU5ERVggKCk7CiAgIExpc3BfT2JqZWN0IG9k ZWFjdGl2YXRlOwpAQCAtNjE0MSw2ICs2MTQ2LDkgQEAgcmVhZF9wcm9jZXNzX291dHB1dCAo TGlzcF9PYmplY3QgcHJvYywgaW50IGNoYW5uZWwpCiAgIFVTRV9TQUZFX0FMTE9DQTsKICAg Y2hhcnMgPSBTQUZFX0FMTE9DQSAoc2l6ZW9mIGNvZGluZy0+Y2FycnlvdmVyICsgcmVhZG1h eCk7CiAKK2RveworICBjYXJyeW92ZXIgPSBwLT5kZWNvZGluZ19jYXJyeW92ZXI7CisKICAg aWYgKGNhcnJ5b3ZlcikKICAgICAvKiBTZWUgdGhlIGNvbW1lbnQgYWJvdmUuICAqLwogICAg IG1lbWNweSAoY2hhcnMsIFNEQVRBIChwLT5kZWNvZGluZ19idWYpLCBjYXJyeW92ZXIpOwpA QCAtNjIyMiwzICs2MjM2LDMgQEAgcmVhZF9wcm9jZXNzX291dHB1dCAoTGlzcF9PYmplY3Qg cHJvYywgaW50IGNoYW5uZWwpCiAgIC8qIE5vdyBzZXQgTkJZVEVTIGhvdyBtYW55IGJ5dGVz IHdlIG11c3QgZGVjb2RlLiAgKi8KICAgbmJ5dGVzICs9IGNhcnJ5b3ZlcjsKIApAQCAtNjIz Myw1ICs2MjQ1LDggQEAKICAgLyogSGFuZGxpbmcgdGhlIHByb2Nlc3Mgb3V0cHV0IHNob3Vs ZCBub3QgZGVhY3RpdmF0ZSB0aGUgbWFyay4gICovCiAgIFZkZWFjdGl2YXRlX21hcmsgPSBv ZGVhY3RpdmF0ZTsKIAorICBubmJ5dGVzICs9IG5ieXRlczsKKyB9IHdoaWxlIChuYnl0ZXMg Pj0gcmVhZG1heCk7CisKICAgU0FGRV9GUkVFX1VOQklORF9UTyAoY291bnQsIFFuaWwpOwot ICByZXR1cm4gbmJ5dGVzOworICByZXR1cm4gbm5ieXRlczsKfQoK --------------X2wXDJQTrLHnoyaJptdb0Mbn--