Building strings with repeated concatenation is somewhat inefficient in
Python; it is better to make a list and glom them all together at the end.
Add a small set of methods to the OutputFormat superclass to manage the
output string, and use them throughout.
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
scripts/lib/kdoc/kdoc_output.py | 185 +++++++++++++++++---------------
1 file changed, 98 insertions(+), 87 deletions(-)
diff --git a/scripts/lib/kdoc/kdoc_output.py b/scripts/lib/kdoc/kdoc_output.py
index ea8914537ba0..d4aabdaa9c51 100644
--- a/scripts/lib/kdoc/kdoc_output.py
+++ b/scripts/lib/kdoc/kdoc_output.py
@@ -73,7 +73,19 @@ class OutputFormat:
self.config = None
self.no_doc_sections = False
- self.data = ""
+ #
+ # Accumulation and management of the output text.
+ #
+ def reset_output(self):
+ self._output = []
+
+ def emit(self, text):
+ """Add a string to out output text"""
+ self._output.append(text)
+
+ def output(self):
+ """Obtain the accumulated output text"""
+ return ''.join(self._output)
def set_config(self, config):
"""
@@ -180,32 +192,31 @@ class OutputFormat:
Handles a single entry from kernel-doc parser
"""
- self.data = ""
-
+ self.reset_output()
dtype = args.type
if dtype == "doc":
self.out_doc(fname, name, args)
- return self.data
+ return self.output()
if not self.check_declaration(dtype, name, args):
- return self.data
+ return self.output()
if dtype == "function":
self.out_function(fname, name, args)
- return self.data
+ return self.output()
if dtype == "enum":
self.out_enum(fname, name, args)
- return self.data
+ return self.output()
if dtype == "typedef":
self.out_typedef(fname, name, args)
- return self.data
+ return self.output()
if dtype in ["struct", "union"]:
self.out_struct(fname, name, args)
- return self.data
+ return self.output()
# Warn if some type requires an output logic
self.config.log.warning("doesn't now how to output '%s' block",
@@ -274,7 +285,7 @@ class RestFormat(OutputFormat):
if self.enable_lineno and ln is not None:
ln += 1
- self.data += f".. LINENO {ln}\n"
+ self.emit(f".. LINENO {ln}\n")
def output_highlight(self, args):
"""
@@ -326,7 +337,7 @@ class RestFormat(OutputFormat):
# Print the output with the line prefix
for line in output.strip("\n").split("\n"):
- self.data += self.lineprefix + line + "\n"
+ self.emit(self.lineprefix + line + "\n")
def out_section(self, args, out_docblock=False):
"""
@@ -343,15 +354,15 @@ class RestFormat(OutputFormat):
if out_docblock:
if not self.out_mode == self.OUTPUT_INCLUDE:
- self.data += f".. _{section}:\n\n"
- self.data += f'{self.lineprefix}**{section}**\n\n'
+ self.emit(f".. _{section}:\n\n")
+ self.emit(f'{self.lineprefix}**{section}**\n\n')
else:
- self.data += f'{self.lineprefix}**{section}**\n\n'
+ self.emit(f'{self.lineprefix}**{section}**\n\n')
self.print_lineno(args.section_start_lines.get(section, 0))
self.output_highlight(text)
- self.data += "\n"
- self.data += "\n"
+ self.emit("\n")
+ self.emit("\n")
def out_doc(self, fname, name, args):
if not self.check_doc(name, args):
@@ -389,41 +400,41 @@ class RestFormat(OutputFormat):
self.print_lineno(ln)
if args.get('typedef') or not args.get('functiontype'):
- self.data += f".. c:macro:: {name}\n\n"
+ self.emit(f".. c:macro:: {name}\n\n")
if args.get('typedef'):
- self.data += " **Typedef**: "
+ self.emit(" **Typedef**: ")
self.lineprefix = ""
self.output_highlight(args.get('purpose', ""))
- self.data += "\n\n**Syntax**\n\n"
- self.data += f" ``{signature}``\n\n"
+ self.emit("\n\n**Syntax**\n\n")
+ self.emit(f" ``{signature}``\n\n")
else:
- self.data += f"``{signature}``\n\n"
+ self.emit(f"``{signature}``\n\n")
else:
- self.data += f".. c:function:: {signature}\n\n"
+ self.emit(f".. c:function:: {signature}\n\n")
if not args.get('typedef'):
self.print_lineno(ln)
self.lineprefix = " "
self.output_highlight(args.get('purpose', ""))
- self.data += "\n"
+ self.emit("\n")
# Put descriptive text into a container (HTML <div>) to help set
# function prototypes apart
self.lineprefix = " "
if args.parameterlist:
- self.data += ".. container:: kernelindent\n\n"
- self.data += f"{self.lineprefix}**Parameters**\n\n"
+ self.emit(".. container:: kernelindent\n\n")
+ self.emit(f"{self.lineprefix}**Parameters**\n\n")
for parameter in args.parameterlist:
parameter_name = KernRe(r'\[.*').sub('', parameter)
dtype = args.parametertypes.get(parameter, "")
if dtype:
- self.data += f"{self.lineprefix}``{dtype}``\n"
+ self.emit(f"{self.lineprefix}``{dtype}``\n")
else:
- self.data += f"{self.lineprefix}``{parameter}``\n"
+ self.emit(f"{self.lineprefix}``{parameter}``\n")
self.print_lineno(args.parameterdesc_start_lines.get(parameter_name, 0))
@@ -432,9 +443,9 @@ class RestFormat(OutputFormat):
args.parameterdescs[parameter_name] != KernelDoc.undescribed:
self.output_highlight(args.parameterdescs[parameter_name])
- self.data += "\n"
+ self.emit("\n")
else:
- self.data += f"{self.lineprefix}*undescribed*\n\n"
+ self.emit(f"{self.lineprefix}*undescribed*\n\n")
self.lineprefix = " "
self.out_section(args)
@@ -445,26 +456,26 @@ class RestFormat(OutputFormat):
oldprefix = self.lineprefix
ln = args.declaration_start_line
- self.data += f"\n\n.. c:enum:: {name}\n\n"
+ self.emit(f"\n\n.. c:enum:: {name}\n\n")
self.print_lineno(ln)
self.lineprefix = " "
self.output_highlight(args.get('purpose', ''))
- self.data += "\n"
+ self.emit("\n")
- self.data += ".. container:: kernelindent\n\n"
+ self.emit(".. container:: kernelindent\n\n")
outer = self.lineprefix + " "
self.lineprefix = outer + " "
- self.data += f"{outer}**Constants**\n\n"
+ self.emit(f"{outer}**Constants**\n\n")
for parameter in args.parameterlist:
- self.data += f"{outer}``{parameter}``\n"
+ self.emit(f"{outer}``{parameter}``\n")
if args.parameterdescs.get(parameter, '') != KernelDoc.undescribed:
self.output_highlight(args.parameterdescs[parameter])
else:
- self.data += f"{self.lineprefix}*undescribed*\n\n"
- self.data += "\n"
+ self.emit(f"{self.lineprefix}*undescribed*\n\n")
+ self.emit("\n")
self.lineprefix = oldprefix
self.out_section(args)
@@ -474,14 +485,14 @@ class RestFormat(OutputFormat):
oldprefix = self.lineprefix
ln = args.declaration_start_line
- self.data += f"\n\n.. c:type:: {name}\n\n"
+ self.emit(f"\n\n.. c:type:: {name}\n\n")
self.print_lineno(ln)
self.lineprefix = " "
self.output_highlight(args.get('purpose', ''))
- self.data += "\n"
+ self.emit("\n")
self.lineprefix = oldprefix
self.out_section(args)
@@ -493,7 +504,7 @@ class RestFormat(OutputFormat):
dtype = args.type
ln = args.declaration_start_line
- self.data += f"\n\n.. c:{dtype}:: {name}\n\n"
+ self.emit(f"\n\n.. c:{dtype}:: {name}\n\n")
self.print_lineno(ln)
@@ -501,20 +512,20 @@ class RestFormat(OutputFormat):
self.lineprefix += " "
self.output_highlight(purpose)
- self.data += "\n"
+ self.emit("\n")
- self.data += ".. container:: kernelindent\n\n"
- self.data += f"{self.lineprefix}**Definition**::\n\n"
+ self.emit(".. container:: kernelindent\n\n")
+ self.emit(f"{self.lineprefix}**Definition**::\n\n")
self.lineprefix = self.lineprefix + " "
declaration = declaration.replace("\t", self.lineprefix)
- self.data += f"{self.lineprefix}{dtype} {name}" + ' {' + "\n"
- self.data += f"{declaration}{self.lineprefix}" + "};\n\n"
+ self.emit(f"{self.lineprefix}{dtype} {name}" + ' {' + "\n")
+ self.emit(f"{declaration}{self.lineprefix}" + "};\n\n")
self.lineprefix = " "
- self.data += f"{self.lineprefix}**Members**\n\n"
+ self.emit(f"{self.lineprefix}**Members**\n\n")
for parameter in args.parameterlist:
if not parameter or parameter.startswith("#"):
continue
@@ -526,15 +537,15 @@ class RestFormat(OutputFormat):
self.print_lineno(args.parameterdesc_start_lines.get(parameter_name, 0))
- self.data += f"{self.lineprefix}``{parameter}``\n"
+ self.emit(f"{self.lineprefix}``{parameter}``\n")
self.lineprefix = " "
self.output_highlight(args.parameterdescs[parameter_name])
self.lineprefix = " "
- self.data += "\n"
+ self.emit("\n")
- self.data += "\n"
+ self.emit("\n")
self.lineprefix = oldprefix
self.out_section(args)
@@ -610,33 +621,33 @@ class ManFormat(OutputFormat):
continue
if line[0] == ".":
- self.data += "\\&" + line + "\n"
+ self.emit("\\&" + line + "\n")
else:
- self.data += line + "\n"
+ self.emit(line + "\n")
def out_doc(self, fname, name, args):
if not self.check_doc(name, args):
return
- self.data += f'.TH "{self.modulename}" 9 "{self.modulename}" "{self.man_date}" "API Manual" LINUX' + "\n"
+ self.emit(f'.TH "{self.modulename}" 9 "{self.modulename}" "{self.man_date}" "API Manual" LINUX' + "\n")
for section, text in args.sections.items():
- self.data += f'.SH "{section}"' + "\n"
+ self.emit(f'.SH "{section}"' + "\n")
self.output_highlight(text)
def out_function(self, fname, name, args):
"""output function in man"""
- self.data += f'.TH "{name}" 9 "{name}" "{self.man_date}" "Kernel Hacker\'s Manual" LINUX' + "\n"
+ self.emit(f'.TH "{name}" 9 "{name}" "{self.man_date}" "Kernel Hacker\'s Manual" LINUX' + "\n")
- self.data += ".SH NAME\n"
- self.data += f"{name} \\- {args['purpose']}\n"
+ self.emit(".SH NAME\n")
+ self.emit(f"{name} \\- {args['purpose']}\n")
- self.data += ".SH SYNOPSIS\n"
+ self.emit(".SH SYNOPSIS\n")
if args.get('functiontype', ''):
- self.data += f'.B "{args["functiontype"]}" {name}' + "\n"
+ self.emit(f'.B "{args["functiontype"]}" {name}' + "\n")
else:
- self.data += f'.B "{name}' + "\n"
+ self.emit(f'.B "{name}' + "\n")
count = 0
parenth = "("
@@ -649,68 +660,68 @@ class ManFormat(OutputFormat):
dtype = args.parametertypes.get(parameter, "")
if function_pointer.match(dtype):
# Pointer-to-function
- self.data += f'".BI "{parenth}{function_pointer.group(1)}" " ") ({function_pointer.group(2)}){post}"' + "\n"
+ self.emit(f'".BI "{parenth}{function_pointer.group(1)}" " ") ({function_pointer.group(2)}){post}"' + "\n")
else:
dtype = KernRe(r'([^\*])$').sub(r'\1 ', dtype)
- self.data += f'.BI "{parenth}{dtype}" "{post}"' + "\n"
+ self.emit(f'.BI "{parenth}{dtype}" "{post}"' + "\n")
count += 1
parenth = ""
if args.parameterlist:
- self.data += ".SH ARGUMENTS\n"
+ self.emit(".SH ARGUMENTS\n")
for parameter in args.parameterlist:
parameter_name = re.sub(r'\[.*', '', parameter)
- self.data += f'.IP "{parameter}" 12' + "\n"
+ self.emit(f'.IP "{parameter}" 12' + "\n")
self.output_highlight(args.parameterdescs.get(parameter_name, ""))
for section, text in args.sections.items():
- self.data += f'.SH "{section.upper()}"' + "\n"
+ self.emit(f'.SH "{section.upper()}"' + "\n")
self.output_highlight(text)
def out_enum(self, fname, name, args):
- self.data += f'.TH "{self.modulename}" 9 "enum {name}" "{self.man_date}" "API Manual" LINUX' + "\n"
+ self.emit(f'.TH "{self.modulename}" 9 "enum {name}" "{self.man_date}" "API Manual" LINUX' + "\n")
- self.data += ".SH NAME\n"
- self.data += f"enum {name} \\- {args['purpose']}\n"
+ self.emit(".SH NAME\n")
+ self.emit(f"enum {name} \\- {args['purpose']}\n")
- self.data += ".SH SYNOPSIS\n"
- self.data += f"enum {name}" + " {\n"
+ self.emit(".SH SYNOPSIS\n")
+ self.emit(f"enum {name}" + " {\n")
count = 0
for parameter in args.parameterlist:
- self.data += f'.br\n.BI " {parameter}"' + "\n"
+ self.emit(f'.br\n.BI " {parameter}"' + "\n")
if count == len(args.parameterlist) - 1:
- self.data += "\n};\n"
+ self.emit("\n};\n")
else:
- self.data += ", \n.br\n"
+ self.emit(", \n.br\n")
count += 1
- self.data += ".SH Constants\n"
+ self.emit(".SH Constants\n")
for parameter in args.parameterlist:
parameter_name = KernRe(r'\[.*').sub('', parameter)
- self.data += f'.IP "{parameter}" 12' + "\n"
+ self.emit(f'.IP "{parameter}" 12' + "\n")
self.output_highlight(args.parameterdescs.get(parameter_name, ""))
for section, text in args.sections.items():
- self.data += f'.SH "{section}"' + "\n"
+ self.emit(f'.SH "{section}"' + "\n")
self.output_highlight(text)
def out_typedef(self, fname, name, args):
module = self.modulename
purpose = args.get('purpose')
- self.data += f'.TH "{module}" 9 "{name}" "{self.man_date}" "API Manual" LINUX' + "\n"
+ self.emit(f'.TH "{module}" 9 "{name}" "{self.man_date}" "API Manual" LINUX' + "\n")
- self.data += ".SH NAME\n"
- self.data += f"typedef {name} \\- {purpose}\n"
+ self.emit(".SH NAME\n")
+ self.emit(f"typedef {name} \\- {purpose}\n")
for section, text in args.sections.items():
- self.data += f'.SH "{section}"' + "\n"
+ self.emit(f'.SH "{section}"' + "\n")
self.output_highlight(text)
def out_struct(self, fname, name, args):
@@ -718,20 +729,20 @@ class ManFormat(OutputFormat):
purpose = args.get('purpose')
definition = args.get('definition')
- self.data += f'.TH "{module}" 9 "{args.type} {name}" "{self.man_date}" "API Manual" LINUX' + "\n"
+ self.emit(f'.TH "{module}" 9 "{args.type} {name}" "{self.man_date}" "API Manual" LINUX' + "\n")
- self.data += ".SH NAME\n"
- self.data += f"{args.type} {name} \\- {purpose}\n"
+ self.emit(".SH NAME\n")
+ self.emit(f"{args.type} {name} \\- {purpose}\n")
# Replace tabs with two spaces and handle newlines
declaration = definition.replace("\t", " ")
declaration = KernRe(r"\n").sub('"\n.br\n.BI "', declaration)
- self.data += ".SH SYNOPSIS\n"
- self.data += f"{args.type} {name} " + "{" + "\n.br\n"
- self.data += f'.BI "{declaration}\n' + "};\n.br\n\n"
+ self.emit(".SH SYNOPSIS\n")
+ self.emit(f"{args.type} {name} " + "{" + "\n.br\n")
+ self.emit(f'.BI "{declaration}\n' + "};\n.br\n\n")
- self.data += ".SH Members\n"
+ self.emit(".SH Members\n")
for parameter in args.parameterlist:
if parameter.startswith("#"):
continue
@@ -741,9 +752,9 @@ class ManFormat(OutputFormat):
if args.parameterdescs.get(parameter_name) == KernelDoc.undescribed:
continue
- self.data += f'.IP "{parameter}" 12' + "\n"
+ self.emit(f'.IP "{parameter}" 12' + "\n")
self.output_highlight(args.parameterdescs.get(parameter_name))
for section, text in args.sections.items():
- self.data += f'.SH "{section}"' + "\n"
+ self.emit(f'.SH "{section}"' + "\n")
self.output_highlight(text)
--
2.49.0Em Wed, 2 Jul 2025 16:35:24 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:
> Building strings with repeated concatenation is somewhat inefficient in
> Python; it is better to make a list and glom them all together at the end.
> Add a small set of methods to the OutputFormat superclass to manage the
> output string, and use them throughout.
>
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
The patch looks good to me. Just a minor nit below.
> ---
> scripts/lib/kdoc/kdoc_output.py | 185 +++++++++++++++++---------------
> 1 file changed, 98 insertions(+), 87 deletions(-)
>
> diff --git a/scripts/lib/kdoc/kdoc_output.py b/scripts/lib/kdoc/kdoc_output.py
> index ea8914537ba0..d4aabdaa9c51 100644
> --- a/scripts/lib/kdoc/kdoc_output.py
> +++ b/scripts/lib/kdoc/kdoc_output.py
> @@ -73,7 +73,19 @@ class OutputFormat:
> self.config = None
> self.no_doc_sections = False
>
> - self.data = ""
> + #
> + # Accumulation and management of the output text.
> + #
> + def reset_output(self):
> + self._output = []
> +
> + def emit(self, text):
> + """Add a string to out output text"""
> + self._output.append(text)
> +
> + def output(self):
> + """Obtain the accumulated output text"""
> + return ''.join(self._output)
I would prefer to use a more Pythonic name for this function:
def __str__(self)
This way, all it takes to get the final string is to use str():
out_str = str(out)
With that:
Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>
> def set_config(self, config):
> """
> @@ -180,32 +192,31 @@ class OutputFormat:
> Handles a single entry from kernel-doc parser
> """
>
> - self.data = ""
> -
> + self.reset_output()
> dtype = args.type
>
> if dtype == "doc":
> self.out_doc(fname, name, args)
> - return self.data
> + return self.output()
>
> if not self.check_declaration(dtype, name, args):
> - return self.data
> + return self.output()
>
> if dtype == "function":
> self.out_function(fname, name, args)
> - return self.data
> + return self.output()
>
> if dtype == "enum":
> self.out_enum(fname, name, args)
> - return self.data
> + return self.output()
>
> if dtype == "typedef":
> self.out_typedef(fname, name, args)
> - return self.data
> + return self.output()
>
> if dtype in ["struct", "union"]:
> self.out_struct(fname, name, args)
> - return self.data
> + return self.output()
>
> # Warn if some type requires an output logic
> self.config.log.warning("doesn't now how to output '%s' block",
> @@ -274,7 +285,7 @@ class RestFormat(OutputFormat):
>
> if self.enable_lineno and ln is not None:
> ln += 1
> - self.data += f".. LINENO {ln}\n"
> + self.emit(f".. LINENO {ln}\n")
>
> def output_highlight(self, args):
> """
> @@ -326,7 +337,7 @@ class RestFormat(OutputFormat):
>
> # Print the output with the line prefix
> for line in output.strip("\n").split("\n"):
> - self.data += self.lineprefix + line + "\n"
> + self.emit(self.lineprefix + line + "\n")
>
> def out_section(self, args, out_docblock=False):
> """
> @@ -343,15 +354,15 @@ class RestFormat(OutputFormat):
>
> if out_docblock:
> if not self.out_mode == self.OUTPUT_INCLUDE:
> - self.data += f".. _{section}:\n\n"
> - self.data += f'{self.lineprefix}**{section}**\n\n'
> + self.emit(f".. _{section}:\n\n")
> + self.emit(f'{self.lineprefix}**{section}**\n\n')
> else:
> - self.data += f'{self.lineprefix}**{section}**\n\n'
> + self.emit(f'{self.lineprefix}**{section}**\n\n')
>
> self.print_lineno(args.section_start_lines.get(section, 0))
> self.output_highlight(text)
> - self.data += "\n"
> - self.data += "\n"
> + self.emit("\n")
> + self.emit("\n")
>
> def out_doc(self, fname, name, args):
> if not self.check_doc(name, args):
> @@ -389,41 +400,41 @@ class RestFormat(OutputFormat):
>
> self.print_lineno(ln)
> if args.get('typedef') or not args.get('functiontype'):
> - self.data += f".. c:macro:: {name}\n\n"
> + self.emit(f".. c:macro:: {name}\n\n")
>
> if args.get('typedef'):
> - self.data += " **Typedef**: "
> + self.emit(" **Typedef**: ")
> self.lineprefix = ""
> self.output_highlight(args.get('purpose', ""))
> - self.data += "\n\n**Syntax**\n\n"
> - self.data += f" ``{signature}``\n\n"
> + self.emit("\n\n**Syntax**\n\n")
> + self.emit(f" ``{signature}``\n\n")
> else:
> - self.data += f"``{signature}``\n\n"
> + self.emit(f"``{signature}``\n\n")
> else:
> - self.data += f".. c:function:: {signature}\n\n"
> + self.emit(f".. c:function:: {signature}\n\n")
>
> if not args.get('typedef'):
> self.print_lineno(ln)
> self.lineprefix = " "
> self.output_highlight(args.get('purpose', ""))
> - self.data += "\n"
> + self.emit("\n")
>
> # Put descriptive text into a container (HTML <div>) to help set
> # function prototypes apart
> self.lineprefix = " "
>
> if args.parameterlist:
> - self.data += ".. container:: kernelindent\n\n"
> - self.data += f"{self.lineprefix}**Parameters**\n\n"
> + self.emit(".. container:: kernelindent\n\n")
> + self.emit(f"{self.lineprefix}**Parameters**\n\n")
>
> for parameter in args.parameterlist:
> parameter_name = KernRe(r'\[.*').sub('', parameter)
> dtype = args.parametertypes.get(parameter, "")
>
> if dtype:
> - self.data += f"{self.lineprefix}``{dtype}``\n"
> + self.emit(f"{self.lineprefix}``{dtype}``\n")
> else:
> - self.data += f"{self.lineprefix}``{parameter}``\n"
> + self.emit(f"{self.lineprefix}``{parameter}``\n")
>
> self.print_lineno(args.parameterdesc_start_lines.get(parameter_name, 0))
>
> @@ -432,9 +443,9 @@ class RestFormat(OutputFormat):
> args.parameterdescs[parameter_name] != KernelDoc.undescribed:
>
> self.output_highlight(args.parameterdescs[parameter_name])
> - self.data += "\n"
> + self.emit("\n")
> else:
> - self.data += f"{self.lineprefix}*undescribed*\n\n"
> + self.emit(f"{self.lineprefix}*undescribed*\n\n")
> self.lineprefix = " "
>
> self.out_section(args)
> @@ -445,26 +456,26 @@ class RestFormat(OutputFormat):
> oldprefix = self.lineprefix
> ln = args.declaration_start_line
>
> - self.data += f"\n\n.. c:enum:: {name}\n\n"
> + self.emit(f"\n\n.. c:enum:: {name}\n\n")
>
> self.print_lineno(ln)
> self.lineprefix = " "
> self.output_highlight(args.get('purpose', ''))
> - self.data += "\n"
> + self.emit("\n")
>
> - self.data += ".. container:: kernelindent\n\n"
> + self.emit(".. container:: kernelindent\n\n")
> outer = self.lineprefix + " "
> self.lineprefix = outer + " "
> - self.data += f"{outer}**Constants**\n\n"
> + self.emit(f"{outer}**Constants**\n\n")
>
> for parameter in args.parameterlist:
> - self.data += f"{outer}``{parameter}``\n"
> + self.emit(f"{outer}``{parameter}``\n")
>
> if args.parameterdescs.get(parameter, '') != KernelDoc.undescribed:
> self.output_highlight(args.parameterdescs[parameter])
> else:
> - self.data += f"{self.lineprefix}*undescribed*\n\n"
> - self.data += "\n"
> + self.emit(f"{self.lineprefix}*undescribed*\n\n")
> + self.emit("\n")
>
> self.lineprefix = oldprefix
> self.out_section(args)
> @@ -474,14 +485,14 @@ class RestFormat(OutputFormat):
> oldprefix = self.lineprefix
> ln = args.declaration_start_line
>
> - self.data += f"\n\n.. c:type:: {name}\n\n"
> + self.emit(f"\n\n.. c:type:: {name}\n\n")
>
> self.print_lineno(ln)
> self.lineprefix = " "
>
> self.output_highlight(args.get('purpose', ''))
>
> - self.data += "\n"
> + self.emit("\n")
>
> self.lineprefix = oldprefix
> self.out_section(args)
> @@ -493,7 +504,7 @@ class RestFormat(OutputFormat):
> dtype = args.type
> ln = args.declaration_start_line
>
> - self.data += f"\n\n.. c:{dtype}:: {name}\n\n"
> + self.emit(f"\n\n.. c:{dtype}:: {name}\n\n")
>
> self.print_lineno(ln)
>
> @@ -501,20 +512,20 @@ class RestFormat(OutputFormat):
> self.lineprefix += " "
>
> self.output_highlight(purpose)
> - self.data += "\n"
> + self.emit("\n")
>
> - self.data += ".. container:: kernelindent\n\n"
> - self.data += f"{self.lineprefix}**Definition**::\n\n"
> + self.emit(".. container:: kernelindent\n\n")
> + self.emit(f"{self.lineprefix}**Definition**::\n\n")
>
> self.lineprefix = self.lineprefix + " "
>
> declaration = declaration.replace("\t", self.lineprefix)
>
> - self.data += f"{self.lineprefix}{dtype} {name}" + ' {' + "\n"
> - self.data += f"{declaration}{self.lineprefix}" + "};\n\n"
> + self.emit(f"{self.lineprefix}{dtype} {name}" + ' {' + "\n")
> + self.emit(f"{declaration}{self.lineprefix}" + "};\n\n")
>
> self.lineprefix = " "
> - self.data += f"{self.lineprefix}**Members**\n\n"
> + self.emit(f"{self.lineprefix}**Members**\n\n")
> for parameter in args.parameterlist:
> if not parameter or parameter.startswith("#"):
> continue
> @@ -526,15 +537,15 @@ class RestFormat(OutputFormat):
>
> self.print_lineno(args.parameterdesc_start_lines.get(parameter_name, 0))
>
> - self.data += f"{self.lineprefix}``{parameter}``\n"
> + self.emit(f"{self.lineprefix}``{parameter}``\n")
>
> self.lineprefix = " "
> self.output_highlight(args.parameterdescs[parameter_name])
> self.lineprefix = " "
>
> - self.data += "\n"
> + self.emit("\n")
>
> - self.data += "\n"
> + self.emit("\n")
>
> self.lineprefix = oldprefix
> self.out_section(args)
> @@ -610,33 +621,33 @@ class ManFormat(OutputFormat):
> continue
>
> if line[0] == ".":
> - self.data += "\\&" + line + "\n"
> + self.emit("\\&" + line + "\n")
> else:
> - self.data += line + "\n"
> + self.emit(line + "\n")
>
> def out_doc(self, fname, name, args):
> if not self.check_doc(name, args):
> return
>
> - self.data += f'.TH "{self.modulename}" 9 "{self.modulename}" "{self.man_date}" "API Manual" LINUX' + "\n"
> + self.emit(f'.TH "{self.modulename}" 9 "{self.modulename}" "{self.man_date}" "API Manual" LINUX' + "\n")
>
> for section, text in args.sections.items():
> - self.data += f'.SH "{section}"' + "\n"
> + self.emit(f'.SH "{section}"' + "\n")
> self.output_highlight(text)
>
> def out_function(self, fname, name, args):
> """output function in man"""
>
> - self.data += f'.TH "{name}" 9 "{name}" "{self.man_date}" "Kernel Hacker\'s Manual" LINUX' + "\n"
> + self.emit(f'.TH "{name}" 9 "{name}" "{self.man_date}" "Kernel Hacker\'s Manual" LINUX' + "\n")
>
> - self.data += ".SH NAME\n"
> - self.data += f"{name} \\- {args['purpose']}\n"
> + self.emit(".SH NAME\n")
> + self.emit(f"{name} \\- {args['purpose']}\n")
>
> - self.data += ".SH SYNOPSIS\n"
> + self.emit(".SH SYNOPSIS\n")
> if args.get('functiontype', ''):
> - self.data += f'.B "{args["functiontype"]}" {name}' + "\n"
> + self.emit(f'.B "{args["functiontype"]}" {name}' + "\n")
> else:
> - self.data += f'.B "{name}' + "\n"
> + self.emit(f'.B "{name}' + "\n")
>
> count = 0
> parenth = "("
> @@ -649,68 +660,68 @@ class ManFormat(OutputFormat):
> dtype = args.parametertypes.get(parameter, "")
> if function_pointer.match(dtype):
> # Pointer-to-function
> - self.data += f'".BI "{parenth}{function_pointer.group(1)}" " ") ({function_pointer.group(2)}){post}"' + "\n"
> + self.emit(f'".BI "{parenth}{function_pointer.group(1)}" " ") ({function_pointer.group(2)}){post}"' + "\n")
> else:
> dtype = KernRe(r'([^\*])$').sub(r'\1 ', dtype)
>
> - self.data += f'.BI "{parenth}{dtype}" "{post}"' + "\n"
> + self.emit(f'.BI "{parenth}{dtype}" "{post}"' + "\n")
> count += 1
> parenth = ""
>
> if args.parameterlist:
> - self.data += ".SH ARGUMENTS\n"
> + self.emit(".SH ARGUMENTS\n")
>
> for parameter in args.parameterlist:
> parameter_name = re.sub(r'\[.*', '', parameter)
>
> - self.data += f'.IP "{parameter}" 12' + "\n"
> + self.emit(f'.IP "{parameter}" 12' + "\n")
> self.output_highlight(args.parameterdescs.get(parameter_name, ""))
>
> for section, text in args.sections.items():
> - self.data += f'.SH "{section.upper()}"' + "\n"
> + self.emit(f'.SH "{section.upper()}"' + "\n")
> self.output_highlight(text)
>
> def out_enum(self, fname, name, args):
> - self.data += f'.TH "{self.modulename}" 9 "enum {name}" "{self.man_date}" "API Manual" LINUX' + "\n"
> + self.emit(f'.TH "{self.modulename}" 9 "enum {name}" "{self.man_date}" "API Manual" LINUX' + "\n")
>
> - self.data += ".SH NAME\n"
> - self.data += f"enum {name} \\- {args['purpose']}\n"
> + self.emit(".SH NAME\n")
> + self.emit(f"enum {name} \\- {args['purpose']}\n")
>
> - self.data += ".SH SYNOPSIS\n"
> - self.data += f"enum {name}" + " {\n"
> + self.emit(".SH SYNOPSIS\n")
> + self.emit(f"enum {name}" + " {\n")
>
> count = 0
> for parameter in args.parameterlist:
> - self.data += f'.br\n.BI " {parameter}"' + "\n"
> + self.emit(f'.br\n.BI " {parameter}"' + "\n")
> if count == len(args.parameterlist) - 1:
> - self.data += "\n};\n"
> + self.emit("\n};\n")
> else:
> - self.data += ", \n.br\n"
> + self.emit(", \n.br\n")
>
> count += 1
>
> - self.data += ".SH Constants\n"
> + self.emit(".SH Constants\n")
>
> for parameter in args.parameterlist:
> parameter_name = KernRe(r'\[.*').sub('', parameter)
> - self.data += f'.IP "{parameter}" 12' + "\n"
> + self.emit(f'.IP "{parameter}" 12' + "\n")
> self.output_highlight(args.parameterdescs.get(parameter_name, ""))
>
> for section, text in args.sections.items():
> - self.data += f'.SH "{section}"' + "\n"
> + self.emit(f'.SH "{section}"' + "\n")
> self.output_highlight(text)
>
> def out_typedef(self, fname, name, args):
> module = self.modulename
> purpose = args.get('purpose')
>
> - self.data += f'.TH "{module}" 9 "{name}" "{self.man_date}" "API Manual" LINUX' + "\n"
> + self.emit(f'.TH "{module}" 9 "{name}" "{self.man_date}" "API Manual" LINUX' + "\n")
>
> - self.data += ".SH NAME\n"
> - self.data += f"typedef {name} \\- {purpose}\n"
> + self.emit(".SH NAME\n")
> + self.emit(f"typedef {name} \\- {purpose}\n")
>
> for section, text in args.sections.items():
> - self.data += f'.SH "{section}"' + "\n"
> + self.emit(f'.SH "{section}"' + "\n")
> self.output_highlight(text)
>
> def out_struct(self, fname, name, args):
> @@ -718,20 +729,20 @@ class ManFormat(OutputFormat):
> purpose = args.get('purpose')
> definition = args.get('definition')
>
> - self.data += f'.TH "{module}" 9 "{args.type} {name}" "{self.man_date}" "API Manual" LINUX' + "\n"
> + self.emit(f'.TH "{module}" 9 "{args.type} {name}" "{self.man_date}" "API Manual" LINUX' + "\n")
>
> - self.data += ".SH NAME\n"
> - self.data += f"{args.type} {name} \\- {purpose}\n"
> + self.emit(".SH NAME\n")
> + self.emit(f"{args.type} {name} \\- {purpose}\n")
>
> # Replace tabs with two spaces and handle newlines
> declaration = definition.replace("\t", " ")
> declaration = KernRe(r"\n").sub('"\n.br\n.BI "', declaration)
>
> - self.data += ".SH SYNOPSIS\n"
> - self.data += f"{args.type} {name} " + "{" + "\n.br\n"
> - self.data += f'.BI "{declaration}\n' + "};\n.br\n\n"
> + self.emit(".SH SYNOPSIS\n")
> + self.emit(f"{args.type} {name} " + "{" + "\n.br\n")
> + self.emit(f'.BI "{declaration}\n' + "};\n.br\n\n")
>
> - self.data += ".SH Members\n"
> + self.emit(".SH Members\n")
> for parameter in args.parameterlist:
> if parameter.startswith("#"):
> continue
> @@ -741,9 +752,9 @@ class ManFormat(OutputFormat):
> if args.parameterdescs.get(parameter_name) == KernelDoc.undescribed:
> continue
>
> - self.data += f'.IP "{parameter}" 12' + "\n"
> + self.emit(f'.IP "{parameter}" 12' + "\n")
> self.output_highlight(args.parameterdescs.get(parameter_name))
>
> for section, text in args.sections.items():
> - self.data += f'.SH "{section}"' + "\n"
> + self.emit(f'.SH "{section}"' + "\n")
> self.output_highlight(text)
Thanks,
Mauro
Em Thu, 10 Jul 2025 08:41:19 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> Em Wed, 2 Jul 2025 16:35:24 -0600
> Jonathan Corbet <corbet@lwn.net> escreveu:
>
> > Building strings with repeated concatenation is somewhat inefficient in
> > Python; it is better to make a list and glom them all together at the end.
> > Add a small set of methods to the OutputFormat superclass to manage the
> > output string, and use them throughout.
> >
> > Signed-off-by: Jonathan Corbet <corbet@lwn.net>
>
> The patch looks good to me. Just a minor nit below.
>
> > ---
> > scripts/lib/kdoc/kdoc_output.py | 185 +++++++++++++++++---------------
> > 1 file changed, 98 insertions(+), 87 deletions(-)
> >
> > diff --git a/scripts/lib/kdoc/kdoc_output.py b/scripts/lib/kdoc/kdoc_output.py
> > index ea8914537ba0..d4aabdaa9c51 100644
> > --- a/scripts/lib/kdoc/kdoc_output.py
> > +++ b/scripts/lib/kdoc/kdoc_output.py
> > @@ -73,7 +73,19 @@ class OutputFormat:
> > self.config = None
> > self.no_doc_sections = False
> >
> > - self.data = ""
> > + #
> > + # Accumulation and management of the output text.
> > + #
> > + def reset_output(self):
> > + self._output = []
> > +
> > + def emit(self, text):
> > + """Add a string to out output text"""
> > + self._output.append(text)
> > +
> > + def output(self):
> > + """Obtain the accumulated output text"""
> > + return ''.join(self._output)
>
> I would prefer to use a more Pythonic name for this function:
>
> def __str__(self)
>
> This way, all it takes to get the final string is to use str():
>
> out_str = str(out)
>
> With that:
>
> Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Hmm... actually, I would code it on a different way, using something like:
class OutputString:
def __init__(self):
"""Initialize internal list"""
self._output = []
# Probably not needed - The code can simply do, instead:
# a = OutputString() to create a new string.
def reset(self):
"""Reset the output text"""
self._output = []
def __add__(self, text):
"""Add a string to out output text"""
if not isinstance(text, str):
raise TypeError("Can only append strings")
self._output.append(text)
return self
def __str__(self):
return ''.join(self._output)
# and, if needed, add a getter/setter:
@property
def data(self):
"""Getter for the current output"""
return ''.join(self._output)
@data.setter
def data(self, new_value):
if isinstance(new_value, str):
self._output = [new_value]
elif isinstance(new_value, list):
self._output = new_value
else:
raise TypeError("Value should be either list or string")
That would allow things like:
out = OutputString()
out = out + "Foo" + " " + "Bar"
print(out)
out = OutputString()
out += "Foo"
out += " "
out += "Bar"
return(str(out))
and won't require much changes at the output logic, and IMO will
provide a cleaner code.
Thanks,
Mauro
Em Thu, 10 Jul 2025 09:13:52 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> Em Thu, 10 Jul 2025 08:41:19 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
>
> > Em Wed, 2 Jul 2025 16:35:24 -0600
> > Jonathan Corbet <corbet@lwn.net> escreveu:
> >
> > > Building strings with repeated concatenation is somewhat inefficient in
> > > Python; it is better to make a list and glom them all together at the end.
> > > Add a small set of methods to the OutputFormat superclass to manage the
> > > output string, and use them throughout.
> > >
> > > Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> >
> > The patch looks good to me. Just a minor nit below.
> >
> > > ---
> > > scripts/lib/kdoc/kdoc_output.py | 185 +++++++++++++++++---------------
> > > 1 file changed, 98 insertions(+), 87 deletions(-)
> > >
> > > diff --git a/scripts/lib/kdoc/kdoc_output.py b/scripts/lib/kdoc/kdoc_output.py
> > > index ea8914537ba0..d4aabdaa9c51 100644
> > > --- a/scripts/lib/kdoc/kdoc_output.py
> > > +++ b/scripts/lib/kdoc/kdoc_output.py
> > > @@ -73,7 +73,19 @@ class OutputFormat:
> > > self.config = None
> > > self.no_doc_sections = False
> > >
> > > - self.data = ""
> > > + #
> > > + # Accumulation and management of the output text.
> > > + #
> > > + def reset_output(self):
> > > + self._output = []
> > > +
> > > + def emit(self, text):
> > > + """Add a string to out output text"""
> > > + self._output.append(text)
> > > +
> > > + def output(self):
> > > + """Obtain the accumulated output text"""
> > > + return ''.join(self._output)
> >
> > I would prefer to use a more Pythonic name for this function:
> >
> > def __str__(self)
> >
> > This way, all it takes to get the final string is to use str():
> >
> > out_str = str(out)
> >
> > With that:
> >
> > Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>
>
> Hmm... actually, I would code it on a different way, using something like:
>
> class OutputString:
> def __init__(self):
> """Initialize internal list"""
> self._output = []
>
> # Probably not needed - The code can simply do, instead:
> # a = OutputString() to create a new string.
> def reset(self):
> """Reset the output text"""
> self._output = []
>
> def __add__(self, text):
> """Add a string to out output text"""
> if not isinstance(text, str):
> raise TypeError("Can only append strings")
> self._output.append(text)
> return self
>
> def __str__(self):
> return ''.join(self._output)
>
> # and, if needed, add a getter/setter:
>
> @property
> def data(self):
> """Getter for the current output"""
> return ''.join(self._output)
>
> @data.setter
> def data(self, new_value):
> if isinstance(new_value, str):
> self._output = [new_value]
> elif isinstance(new_value, list):
> self._output = new_value
> else:
> raise TypeError("Value should be either list or string")
>
> That would allow things like:
>
> out = OutputString()
> out = out + "Foo" + " " + "Bar"
> print(out)
>
> out = OutputString()
> out += "Foo"
> out += " "
> out += "Bar"
> return(str(out))
>
> and won't require much changes at the output logic, and IMO will
> provide a cleaner code.
>
> Thanks,
> Mauro
Heh, on those times where LLM can quickly code trivial things for us,
I actually decided to test 3 different variants:
- using string +=
- using list append
- using __add__
- using __iadd__
Except if the LLM-generated did something wrong (I double checked, and
was unable to find any issues), the results on Python 3.13.5 are:
$ /tmp/bench.py
Benchmarking 1,000 ops × 1000 strings = 1,000,000 appends
Run str+= ExplicitList __add__ __iadd__
------------------------------------------------------------
1 25.26 29.44 53.42 50.71
2 29.34 29.35 53.45 50.61
3 29.44 29.56 53.41 50.67
4 29.28 29.23 53.26 50.64
5 29.28 29.20 45.90 40.47
6 23.53 23.62 42.74 40.61
7 23.43 23.76 42.97 40.78
8 23.51 23.59 42.67 40.61
9 23.43 23.52 42.77 40.72
10 23.53 23.54 42.78 40.67
11 23.83 23.63 42.98 40.87
12 23.49 23.45 42.67 40.53
13 23.43 23.69 42.75 40.66
14 23.47 23.49 42.70 40.56
15 23.44 23.63 42.72 40.52
16 23.51 23.56 42.65 40.66
17 23.48 23.60 42.86 40.81
18 23.67 23.53 42.73 40.59
19 23.75 23.62 42.78 40.58
20 23.68 23.55 42.77 40.54
21 23.65 23.67 42.76 40.59
22 23.73 23.49 42.78 40.61
23 23.61 23.59 42.78 40.58
24 23.66 23.51 42.73 40.55
------------------------------------------------------------
Avg 24.60 24.78 44.67 42.30
Summary:
ExplicitList : 100.74% slower than str+=
__add__ : 181.56% slower than str+=
__iadd__ : 171.93% slower than str+=
(running it a couple of times sometimes it sometimes show list
addition a little bit better, bu all at the +/- 1% range)
In practice, it means that my suggestion of using __add__ (or even
using the __iadd__ variant) was not good, but it also showed
that Python 3.13 implementation is actually very efficient
with str += operations.
With that, I would just drop this patch, as the performance is
almost identical, and using "emit()" instead of "+=" IMO makes
the code less clear.
-
Btw, with Python 3.9, "".join(list) is a lot worse than str += :
$ python3.9 /tmp/bench.py
Benchmarking 1,000 ops × 1000 strings = 1,000,000 appends
Run str+= ExplicitList __add__ __iadd__
------------------------------------------------------------
1 28.27 87.24 96.03 88.81
2 32.76 87.35 87.40 88.92
3 32.69 85.98 73.01 70.87
4 26.28 69.80 70.62 71.90
5 27.21 70.54 71.04 72.00
6 27.77 70.06 70.22 70.92
7 27.03 69.75 70.30 70.89
8 33.31 72.63 70.57 70.59
9 26.33 70.15 70.27 70.97
10 26.29 69.84 71.60 70.94
11 26.59 69.60 70.16 71.26
12 26.38 69.57 71.64 70.95
13 26.41 69.89 70.11 70.85
14 26.38 69.86 70.36 70.93
15 26.43 69.57 70.18 70.90
16 26.38 70.04 70.26 71.19
17 26.40 70.02 80.50 71.01
18 26.41 71.74 80.39 71.90
19 28.06 69.60 71.95 70.88
20 28.28 69.90 71.12 71.07
21 26.34 69.74 72.42 71.02
22 26.33 69.86 70.25 70.97
23 26.40 69.78 71.64 71.10
24 26.44 69.73 70.23 70.83
------------------------------------------------------------
Avg 27.55 72.18 73.43 72.57
Summary:
ExplicitList : 262.00% slower than str+=
__add__ : 266.54% slower than str+=
__iadd__ : 263.42% slower than str+=
Thanks,
Mauro
---
#!/usr/bin/env python3
import timeit
class ExplicitList:
def __init__(self):
self._output = []
def emit(self, text):
self._output.append(text)
def output(self):
return ''.join(self._output)
class OutputStringAdd:
def __init__(self):
self._output = []
def __add__(self, text):
self._output.append(text)
return self
def __str__(self):
return ''.join(self._output)
class OutputStringIAdd:
def __init__(self):
self._output = []
def __iadd__(self, text):
self._output.append(text)
return self
def __str__(self):
return ''.join(self._output)
def calculate_comparison(base_time, compare_time):
"""Returns tuple of (is_faster, percentage)"""
if compare_time < base_time:
return (True, (1 - compare_time/base_time)*100)
else:
return (False, (compare_time/base_time)*100)
def benchmark():
N = 1000 # Operations
STRINGS_PER_OP = 1000
REPEATS = 24
# Generate test data (1000 unique 10-character strings)
test_strings = [f"string_{i:03d}" for i in range(STRINGS_PER_OP)]
print(f"Benchmarking {N:,} ops × {STRINGS_PER_OP} strings = {N*STRINGS_PER_OP:,} appends\n")
headers = ['Run', 'str+=', 'ExplicitList', '__add__', '__iadd__']
print(f"{headers[0]:<6} {headers[1]:<12} {headers[2]:<12} {headers[3]:<12} {headers[4]:<12}")
print("-" * 60)
results = []
for i in range(REPEATS):
# Benchmark normal string +=
t_str = timeit.timeit(
'result = ""\nfor s in test_strings: result += s',
globals={'test_strings': test_strings},
number=N
) * 1000
# Benchmark ExplicitList
t_explicit = timeit.timeit(
'obj = ExplicitList()\nfor s in test_strings: obj.emit(s)',
globals={'test_strings': test_strings, 'ExplicitList': ExplicitList},
number=N
) * 1000
# Benchmark __add__ version
t_add = timeit.timeit(
'obj = OutputStringAdd()\nfor s in test_strings: obj += s',
globals={'test_strings': test_strings, 'OutputStringAdd': OutputStringAdd},
number=N
) * 1000
# Benchmark __iadd__ version
t_iadd = timeit.timeit(
'obj = OutputStringIAdd()\nfor s in test_strings: obj += s',
globals={'test_strings': test_strings, 'OutputStringIAdd': OutputStringIAdd},
number=N
) * 1000
results.append((t_str, t_explicit, t_add, t_iadd))
print(f"{i+1:<6} {t_str:<12.2f} {t_explicit:<12.2f} {t_add:<12.2f} {t_iadd:<12.2f}")
# Calculate averages
avg_str = sum(r[0] for r in results) / REPEATS
avg_explicit = sum(r[1] for r in results) / REPEATS
avg_add = sum(r[2] for r in results) / REPEATS
avg_iadd = sum(r[3] for r in results) / REPEATS
print("-" * 60)
print(f"{'Avg':<6} {avg_str:<12.2f} {avg_explicit:<12.2f} {avg_add:<12.2f} {avg_iadd:<12.2f}")
print()
print("Summary:")
# Calculate and print comparisons
for name, time in [("ExplicitList", avg_explicit),
("__add__", avg_add),
("__iadd__", avg_iadd)]:
is_faster, percentage = calculate_comparison(avg_str, time)
if is_faster:
print(f"{name:<12} : {percentage:.2f}% faster than str+=")
else:
print(f"{name:<12} : {percentage:.2f}% slower than str+=")
if __name__ == "__main__":
benchmark()
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes: > With that, I would just drop this patch, as the performance is > almost identical, and using "emit()" instead of "+=" IMO makes > the code less clear. I've dropped the patch - for now - but I really disagree with the latter part of that sentence. It is far better, IMO, to encapsulate the construction of our output rather than spreading vast numbers of direct string concatenations throughout the code. So this one will likely be back in a different form :) Thanks, jon
Em Thu, 10 Jul 2025 17:30:20 -0600 Jonathan Corbet <corbet@lwn.net> escreveu: > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes: > > > With that, I would just drop this patch, as the performance is > > almost identical, and using "emit()" instead of "+=" IMO makes > > the code less clear. > > I've dropped the patch - for now - but I really disagree with the latter > part of that sentence. It is far better, IMO, to encapsulate the > construction of our output rather than spreading vast numbers of direct > string concatenations throughout the code. So this one will likely be > back in a different form :) The main concern was related to performance penalty - as based on the latest test results, Pyhon currently handles very poorly list concat (30% to 200% slower at the latest test results). Yet, at least for me with my C-trained brain parsing, I find "=+" a lot easier to understand than some_function(). Btw, IMHO Python is not particularly great with names for concat/accumulate commands. For list, it is append(), for set it is add(). Yet, "+=" is almost universal (from standard types, only sets don't accept it, using, instead, "|=", which kind of makes sense). Adding a function naming emit() - at least for me - requires an extra brain processing time to remember that emit is actually a function that doesn't produce any emission: it just stores data for a future output - that may even not happen if one calls the script with "--none". Thanks, Mauro
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes: > Em Thu, 10 Jul 2025 17:30:20 -0600 > Jonathan Corbet <corbet@lwn.net> escreveu: > >> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes: >> >> > With that, I would just drop this patch, as the performance is >> > almost identical, and using "emit()" instead of "+=" IMO makes >> > the code less clear. >> >> I've dropped the patch - for now - but I really disagree with the latter >> part of that sentence. It is far better, IMO, to encapsulate the >> construction of our output rather than spreading vast numbers of direct >> string concatenations throughout the code. So this one will likely be >> back in a different form :) > > The main concern was related to performance penalty - as based on > the latest test results, Pyhon currently handles very poorly list > concat (30% to 200% slower at the latest test results). Yes, I understood that part > Yet, at least for me with my C-trained brain parsing, I find "=+" a > lot easier to understand than some_function(). > > Btw, IMHO Python is not particularly great with names for concat/accumulate > commands. For list, it is append(), for set it is add(). Yet, "+=" is almost > universal (from standard types, only sets don't accept it, using, > instead, "|=", which kind of makes sense). > > Adding a function naming emit() - at least for me - requires an extra brain > processing time to remember that emit is actually a function that doesn't > produce any emission: it just stores data for a future output - that may > even not happen if one calls the script with "--none". OK, I'll ponder on a different name :) Perhaps the new not_emit() could even be aware of --none and just drop the data on the floor. Thanks, jon
Em Fri, 11 Jul 2025 06:49:26 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
>
> > Em Thu, 10 Jul 2025 17:30:20 -0600
> > Jonathan Corbet <corbet@lwn.net> escreveu:
> >
> >> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
> >>
> >> > With that, I would just drop this patch, as the performance is
> >> > almost identical, and using "emit()" instead of "+=" IMO makes
> >> > the code less clear.
> >>
> >> I've dropped the patch - for now - but I really disagree with the latter
> >> part of that sentence. It is far better, IMO, to encapsulate the
> >> construction of our output rather than spreading vast numbers of direct
> >> string concatenations throughout the code. So this one will likely be
> >> back in a different form :)
> >
> > The main concern was related to performance penalty - as based on
> > the latest test results, Pyhon currently handles very poorly list
> > concat (30% to 200% slower at the latest test results).
>
> Yes, I understood that part
>
> > Yet, at least for me with my C-trained brain parsing, I find "=+" a
> > lot easier to understand than some_function().
> >
> > Btw, IMHO Python is not particularly great with names for concat/accumulate
> > commands. For list, it is append(), for set it is add(). Yet, "+=" is almost
> > universal (from standard types, only sets don't accept it, using,
> > instead, "|=", which kind of makes sense).
> >
> > Adding a function naming emit() - at least for me - requires an extra brain
> > processing time to remember that emit is actually a function that doesn't
> > produce any emission: it just stores data for a future output - that may
> > even not happen if one calls the script with "--none".
>
> OK, I'll ponder on a different name :)
I'm fine with that.
> Perhaps the new not_emit() could even be aware of --none and just drop
> the data on the floor.
The code already does that on a much more optimized way. This
is actually one of the improvements over the Perl version: we
don't need to implement anything special for none.
When --none is passed, the code sets out_style = OutputFormat(),
which is pretty much an abstract class that doesn't do any output
at all, and from where the ManOutput and Restformat classes
are inherited.
It only does two things:
- Applying filters, in order to filter-out warnings from things
according with --import/--internal/--function arguments;
- print warnings for symbols after filtering them, with:
def out_warnings(self, args):
"""
Output warnings for identifiers that will be displayed.
"""
warnings = args.get('warnings', [])
for log_msg in warnings:
self.config.warning(log_msg)
So, there's no emit()/no_emit()/print()... there. All output
types do nothing:
# Virtual methods to be overridden by inherited classes
# At the base class, those do nothing.
def out_doc(self, fname, name, args):
"""Outputs a DOC block"""
def out_function(self, fname, name, args):
"""Outputs a function"""
def out_enum(self, fname, name, args):
"""Outputs an enum"""
def out_typedef(self, fname, name, args):
"""Outputs a typedef"""
def out_struct(self, fname, name, args):
"""Outputs a struct"""
Regards,
Mauro
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes: >> Perhaps the new not_emit() could even be aware of --none and just drop >> the data on the floor. > > The code already does that on a much more optimized way. This > is actually one of the improvements over the Perl version: we > don't need to implement anything special for none. So your comment on emit() being mis-named for the --none case didn't actually apply... :) jon
Em Thu, 10 Jul 2025 10:19:31 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> Em Thu, 10 Jul 2025 09:13:52 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
>
> Heh, on those times where LLM can quickly code trivial things for us,
> I actually decided to test 3 different variants:
>
> - using string +=
> - using list append
> - using __add__
> - using __iadd__
Manually reorganized the LLM-generated code, in order to get more
precise results. Script enclosed at the end.
$ for i in python3.9 python3.13 python3.13t; do echo " $i:"; $i /tmp/bench.py 100000 10 1; $i /tmp/bench.py 1000 1000 1; done
python3.9:
10 strings in a loop with 100000 interactions, repeating 24 times
str += : time: 25.21
list join : time: 72.65: 188.18% slower than str +=
__add__ : time: 71.82: 184.88% slower than str +=
__iadd__ : time: 67.84: 169.09% slower than str +=
1000 strings in a loop with 1000 interactions, repeating 24 times
str += : time: 24.29
list join : time: 58.76: 141.88% slower than str +=
__add__ : time: 58.68: 141.54% slower than str +=
__iadd__ : time: 55.48: 128.37% slower than str +=
python3.13:
10 strings in a loop with 100000 interactions, repeating 24 times
str += : time: 28.01
list join : time: 32.46: 15.91% slower than str +=
__add__ : time: 52.56: 87.66% slower than str +=
__iadd__ : time: 58.69: 109.55% slower than str +=
1000 strings in a loop with 1000 interactions, repeating 24 times
str += : time: 22.03
list join : time: 23.38: 6.12% slower than str +=
__add__ : time: 44.25: 100.86% slower than str +=
__iadd__ : time: 40.70: 84.74% slower than str +=
python3.13t:
10 strings in a loop with 100000 interactions, repeating 24 times
str += : time: 25.65
list join : time: 74.95: 192.18% slower than str +=
__add__ : time: 83.04: 223.71% slower than str +=
__iadd__ : time: 79.07: 208.23% slower than str +=
1000 strings in a loop with 1000 interactions, repeating 24 times
str += : time: 57.39
list join : time: 62.31: 8.58% slower than str +=
__add__ : time: 70.65: 23.10% slower than str +=
__iadd__ : time: 68.67: 19.65% slower than str +=
From the above:
- It is not worth applying patch 12/12 as it makes the code slower;
- Python 3.13t (no-GIL version) had very bad results. It seems it
still requires optimization;
- Python 3.9 is a lot worse (140% to 190%) when using list append;
- when there are not many concats, Python 3.13 is about 15% slower
with lists than concat strings. It only approaches str concat
when the number of concats is high.
With the above, clearly str += is faster than list append.
So, except if I did something wrong on this benchmark script, please
don't apply patch 12/12.
Regards,
Mauro
---
Benchmark code:
#!/usr/bin/env python3
import argparse
import time
import sys
def benchmark_str_concat(test_strings, n_ops):
start = time.time()
for _ in range(n_ops):
result = ""
for s in test_strings:
result += s
return (time.time() - start) * 1000
def benchmark_explicit_list(test_strings, n_ops):
class ExplicitList:
def __init__(self):
self._output = []
def emit(self, text):
self._output.append(text)
def output(self):
return ''.join(self._output)
start = time.time()
for _ in range(n_ops):
obj = ExplicitList()
for s in test_strings:
obj.emit(s)
return (time.time() - start) * 1000
def benchmark_add_overload(test_strings, n_ops):
class OutputStringAdd:
def __init__(self):
self._output = []
def __add__(self, text):
self._output.append(text)
return self
def __str__(self):
return ''.join(self._output)
start = time.time()
for _ in range(n_ops):
obj = OutputStringAdd()
for s in test_strings:
obj += s
return (time.time() - start) * 1000
def benchmark_iadd_overload(test_strings, n_ops):
class OutputStringIAdd:
def __init__(self):
self._output = []
def __iadd__(self, text):
self._output.append(text)
return self
def __str__(self):
return ''.join(self._output)
start = time.time()
for _ in range(n_ops):
obj = OutputStringIAdd()
for s in test_strings:
obj += s
return (time.time() - start) * 1000
def calculate_comparison(base_time, compare_time):
if compare_time < base_time:
return (True, (1 - compare_time/base_time)*100)
return (False, (compare_time/base_time - 1)*100)
def benchmark(num_reps, strings_per_run, repeats, detail):
test_strings = [f"string_{i:03d}" for i in range(strings_per_run)]
# Create benchmark execution order list
benchmarks = [
("str +=", benchmark_str_concat),
("list join", benchmark_explicit_list),
("__add__", benchmark_add_overload),
("__iadd__", benchmark_iadd_overload)
]
# Use all possible permutations of benchmark order to reduce any
# noise due to CPU caches
all_orders = [
(0, 1, 2, 3), (0, 1, 3, 2), (0, 2, 1, 3), (0, 2, 3, 1),
(0, 3, 1, 2), (0, 3, 2, 1), (1, 0, 2, 3), (1, 0, 3, 2),
(1, 2, 0, 3), (1, 2, 3, 0), (1, 3, 0, 2), (1, 3, 2, 0),
(2, 0, 1, 3), (2, 0, 3, 1), (2, 1, 0, 3), (2, 1, 3, 0),
(2, 3, 0, 1), (2, 3, 1, 0), (3, 0, 1, 2), (3, 0, 2, 1),
(3, 1, 0, 2), (3, 1, 2, 0), (3, 2, 0, 1), (3, 2, 1, 0)
]
results = {}
for name, _ in benchmarks:
results[name] = 0
# Warm-up phase to reduce caching issues
for name, fn in benchmarks:
fn(test_strings, 1)
n_repeats = len(all_orders) * repeats
print(f" {strings_per_run} strings in a loop with {num_reps} interactions, repeating {n_repeats} times")
# Actual benchmark starts here
i = 0
if detail:
headers = ['Run'] + [name for name, _ in benchmarks]
print()
print(f"\t{headers[0]:<6} {headers[1]:<12} {headers[2]:<12} {headers[3]:<12} {headers[4]:<12}")
print("\t" + "-" * 60)
for _ in range(repeats):
# Shuffle execution order each run
for order in all_orders:
run_results = {}
for idx in order:
name, func = benchmarks[idx]
run_results[name] = func(test_strings, num_reps)
results[name] += run_results[name]
if detail:
# Output results in consistent order
print(f"\t{i+1:<6}", end=" ")
for name, _ in benchmarks:
print(f"{run_results[name]:<12.2f}", end=" ")
print()
i += 1
avg_results = {}
for name, _ in benchmarks:
avg_results[name] = results[name] / repeats / len(all_orders)
if detail:
print("\t" + "-" * 60)
print(f"\t ", end=" ")
for name, _ in benchmarks:
print(f"{avg_results[name]:<12.2f}", end=" ")
print()
print()
ref = benchmarks.pop(0)
print(f"\t{ref[0]:<12} : time: {avg_results[ref[0]]:3.2f}")
for name, _ in benchmarks:
is_faster, percentage = calculate_comparison(avg_results[ref[0]], avg_results[name])
direction = "faster" if is_faster else "slower"
print(f"\t{name:<12} : time: {avg_results[name]:3.2f}: {percentage:3.2f}% {direction} than {ref[0]}")
if __name__ == "__main__":
parser = argparse.ArgumentParser()
parser.add_argument('-d', '--detail', action='store_true',
help='Enable detailed output')
args, remaining = parser.parse_known_args()
# Then handle the positional arguments manually
if len(remaining) != 3:
print(f"Usage: {sys.argv[0]} [-d] <num_repetitions> <strings_per_op> <repeats>")
sys.exit(1)
num_reps = int(remaining[0])
strings_per_op = int(remaining[1])
repeats = int(remaining[2])
num_reps = int(sys.argv[1])
strings_per_op = int(sys.argv[2])
repeats = int(sys.argv[3])
benchmark(num_reps, strings_per_op, repeats, args.detail)
Em Thu, 10 Jul 2025 12:10:33 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> From the above:
>
> - It is not worth applying patch 12/12 as it makes the code slower;
> - Python 3.13t (no-GIL version) had very bad results. It seems it
> still requires optimization;
> - Python 3.9 is a lot worse (140% to 190%) when using list append;
> - when there are not many concats, Python 3.13 is about 15% slower
> with lists than concat strings. It only approaches str concat
> when the number of concats is high.
>
> With the above, clearly str += is faster than list append.
>
> So, except if I did something wrong on this benchmark script, please
> don't apply patch 12/12.
And I did: I forgot the final line at the concat code to get the
result as strings.
For explicit list:
result = obj.output()
For implicit ones:
result = str(obj)
Yet, the conclusion is similar. With Python 3.13:
$ for i in python3.13; do for j in 1 10 100 1000; do $i /tmp/bench.py $((1000000/$j)) $j 1; done; done
1 strings in a loop with 1000000 interactions, repeating 24 times
str += : time: 41.42
list join : time: 127.33: 207.42% slower than str +=
10 strings in a loop with 100000 interactions, repeating 24 times
str += : time: 27.15
list join : time: 39.19: 44.36% slower than str +=
100 strings in a loop with 10000 interactions, repeating 24 times
str += : time: 24.84
list join : time: 30.70: 23.57% slower than str +=
1000 strings in a loop with 1000 interactions, repeating 24 times
str += : time: 21.84
list join : time: 27.85: 27.50% slower than str +=
Explict list concat was between ~30% to ~200% worse than str concat.
Thanks,
Mauro
---
#!/usr/bin/env python3
import argparse
import time
import sys
def benchmark_str_concat(test_strings, n_ops):
start = time.time()
for _ in range(n_ops):
result = ""
for s in test_strings:
result += s
return (time.time() - start) * 1000
def benchmark_explicit_list(test_strings, n_ops):
class ExplicitList:
def __init__(self):
self._output = []
def emit(self, text):
self._output.append(text)
def output(self):
return ''.join(self._output)
start = time.time()
for _ in range(n_ops):
obj = ExplicitList()
for s in test_strings:
obj.emit(s)
result = obj.output()
return (time.time() - start) * 1000
def benchmark_add_overload(test_strings, n_ops):
class OutputStringAdd:
def __init__(self):
self._output = []
def __add__(self, text):
self._output.append(text)
return self
def __str__(self):
return ''.join(self._output)
start = time.time()
for _ in range(n_ops):
obj = OutputStringAdd()
for s in test_strings:
obj += s
result = str(obj)
return (time.time() - start) * 1000
def benchmark_iadd_overload(test_strings, n_ops):
class OutputStringIAdd:
def __init__(self):
self._output = []
def __iadd__(self, text):
self._output.append(text)
return self
def __str__(self):
return ''.join(self._output)
start = time.time()
for _ in range(n_ops):
obj = OutputStringIAdd()
for s in test_strings:
obj += s
result = str(obj)
return (time.time() - start) * 1000
def calculate_comparison(base_time, compare_time):
if compare_time < base_time:
return (True, (1 - compare_time/base_time)*100)
return (False, (compare_time/base_time - 1)*100)
def benchmark(num_reps, strings_per_run, repeats, detail):
test_strings = [f"string_{i:03d}" for i in range(strings_per_run)]
# Create benchmark execution order list
benchmarks = [
("str +=", benchmark_str_concat),
("list join", benchmark_explicit_list),
("__add__", benchmark_add_overload),
("__iadd__", benchmark_iadd_overload)
]
# Use all possible permutations of benchmark order to reduce any
# noise due to CPU caches
all_orders = [
(0, 1, 2, 3), (0, 1, 3, 2), (0, 2, 1, 3), (0, 2, 3, 1),
(0, 3, 1, 2), (0, 3, 2, 1), (1, 0, 2, 3), (1, 0, 3, 2),
(1, 2, 0, 3), (1, 2, 3, 0), (1, 3, 0, 2), (1, 3, 2, 0),
(2, 0, 1, 3), (2, 0, 3, 1), (2, 1, 0, 3), (2, 1, 3, 0),
(2, 3, 0, 1), (2, 3, 1, 0), (3, 0, 1, 2), (3, 0, 2, 1),
(3, 1, 0, 2), (3, 1, 2, 0), (3, 2, 0, 1), (3, 2, 1, 0)
]
results = {}
for name, _ in benchmarks:
results[name] = 0
# Warm-up phase to reduce caching issues
for name, fn in benchmarks:
fn(test_strings, 1)
n_repeats = len(all_orders) * repeats
print(f" {strings_per_run} strings in a loop with {num_reps} interactions, repeating {n_repeats} times")
# Actual benchmark starts here
i = 0
if detail:
headers = ['Run'] + [name for name, _ in benchmarks]
print()
print(f"\t{headers[0]:<6} {headers[1]:<12} {headers[2]:<12} {headers[3]:<12} {headers[4]:<12}")
print("\t" + "-" * 60)
for _ in range(repeats):
# Shuffle execution order each run
for order in all_orders:
run_results = {}
for idx in order:
name, func = benchmarks[idx]
run_results[name] = func(test_strings, num_reps)
results[name] += run_results[name]
if detail:
# Output results in consistent order
print(f"\t{i+1:<6}", end=" ")
for name, _ in benchmarks:
print(f"{run_results[name]:<12.2f}", end=" ")
print()
i += 1
avg_results = {}
for name, _ in benchmarks:
avg_results[name] = results[name] / repeats / len(all_orders)
if detail:
print("\t" + "-" * 60)
print(f"\t ", end=" ")
for name, _ in benchmarks:
print(f"{avg_results[name]:<12.2f}", end=" ")
print()
print()
ref = benchmarks.pop(0)
print(f"\t{ref[0]:<12} : time: {avg_results[ref[0]]:3.2f}")
for name, _ in benchmarks:
is_faster, percentage = calculate_comparison(avg_results[ref[0]], avg_results[name])
direction = "faster" if is_faster else "slower"
print(f"\t{name:<12} : time: {avg_results[name]:3.2f}: {percentage:3.2f}% {direction} than {ref[0]}")
if __name__ == "__main__":
parser = argparse.ArgumentParser()
parser.add_argument('-d', '--detail', action='store_true',
help='Enable detailed output')
args, remaining = parser.parse_known_args()
# Then handle the positional arguments manually
if len(remaining) != 3:
print(f"Usage: {sys.argv[0]} [-d] <num_repetitions> <strings_per_op> <repeats>")
sys.exit(1)
num_reps = int(remaining[0])
strings_per_op = int(remaining[1])
repeats = int(remaining[2])
num_reps = int(sys.argv[1])
strings_per_op = int(sys.argv[2])
repeats = int(sys.argv[3])
benchmark(num_reps, strings_per_op, repeats, args.detail)
Em Thu, 10 Jul 2025 12:31:55 +0200 Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu: > Em Thu, 10 Jul 2025 12:10:33 +0200 > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu: > > > From the above: > > > > - It is not worth applying patch 12/12 as it makes the code slower; > > - Python 3.13t (no-GIL version) had very bad results. It seems it > > still requires optimization; > > - Python 3.9 is a lot worse (140% to 190%) when using list append; > > - when there are not many concats, Python 3.13 is about 15% slower > > with lists than concat strings. It only approaches str concat > > when the number of concats is high. > > > > With the above, clearly str += is faster than list append. > > > > So, except if I did something wrong on this benchmark script, please > > don't apply patch 12/12. > > And I did: I forgot the final line at the concat code to get the > result as strings. > > For explicit list: > result = obj.output() > > For implicit ones: > result = str(obj) > > Yet, the conclusion is similar. With Python 3.13: > > $ for i in python3.13; do for j in 1 10 100 1000; do $i /tmp/bench.py $((1000000/$j)) $j 1; done; done > 1 strings in a loop with 1000000 interactions, repeating 24 times > str += : time: 41.42 > list join : time: 127.33: 207.42% slower than str += > 10 strings in a loop with 100000 interactions, repeating 24 times > str += : time: 27.15 > list join : time: 39.19: 44.36% slower than str += > 100 strings in a loop with 10000 interactions, repeating 24 times > str += : time: 24.84 > list join : time: 30.70: 23.57% slower than str += > 1000 strings in a loop with 1000 interactions, repeating 24 times > str += : time: 21.84 > list join : time: 27.85: 27.50% slower than str += > > Explict list concat was between ~30% to ~200% worse than str concat. Looking for an explanation, PEP 509 and PEP 393 did Short String Optimization and Inline Caching. This was applied Python 3.6, and Python 3.13 came with extra string optimizations. On the other hand, lists do more memory allocations, have some logic to extend list growth and has an extra concat loop. With that, contrary to popular belief, it sounds that str concat are nowadays faster. Thanks, Mauro
© 2016 - 2025 Red Hat, Inc.