Re: [PATCH v5 01/12] pkt-line: eliminate the need for static buffer in packet_write_gently()

Jeff King [off-list ref] writes:

On Tue, Mar 09, 2021 at 03:48:40PM -0800, Junio C Hamano wrote:

quoted

"Jeff Hostetler via GitGitGadget" [off-list ref] writes:

quoted

+	/*
+	 * Write the header and the buffer in 2 parts so that we do not need
+	 * to allocate a buffer or rely on a static buffer.  This avoids perf
+	 * and multi-threading issues.
+	 */

I understand "multi-threading issues" (i.e. let's not have too much
stuff on the stack), but what issue around "perf" are we worried
about?
 ...

Yeah, this came from my suggestion. My gut feeling is that it isn't
likely to matter, but I'd much rather solve any performance problem we
find using writev(), which would be pretty easy to emulate with a
wrapper for systems that lack it.

I too had writev() in mind when I said "can fix it locally", so we
are on the same page, which is good.

So "this avoid multi-threading issues" without mentioning "perf and"
would be more appropriate?

Thanks.

On Thu, Mar 11, 2021 at 12:32:29PM -0800, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

On Tue, Mar 09, 2021 at 03:48:40PM -0800, Junio C Hamano wrote:

quoted

"Jeff Hostetler via GitGitGadget" [off-list ref] writes:

quoted

+	/*
+	 * Write the header and the buffer in 2 parts so that we do not need
+	 * to allocate a buffer or rely on a static buffer.  This avoids perf
+	 * and multi-threading issues.
+	 */

I understand "multi-threading issues" (i.e. let's not have too much
stuff on the stack), but what issue around "perf" are we worried
about?
 ...

Yeah, this came from my suggestion. My gut feeling is that it isn't
likely to matter, but I'd much rather solve any performance problem we
find using writev(), which would be pretty easy to emulate with a
wrapper for systems that lack it.

I too had writev() in mind when I said "can fix it locally", so we
are on the same page, which is good.

So "this avoid multi-threading issues" without mentioning "perf and"
would be more appropriate?

IMHO yes. I think "avoid perf issues" is probably answering the "why not
just heap-allocate the buffer" question. But that makes sense in the
commit message, not in a comment.

-Peff