On Tue, Oct 24, 2023 at 06:15:06PM +0200, Patrice Dumas wrote: > Maybe it is because of the C code that is run in parallel with perl code? > That would be my first guess. Hopefully, the default will be not to run > C and perl in parallel for the release. Maybe you could compare with > a run with > TEXINFO_XS=require > TEXINFO_XS_CONVERT=1 > which would use only C if possible. > > But there could still be places where both are still run, but hopefully > should be temporary. > > Also, running the C code requires some more perl code, mainly to encode > strings in UTF-8 before passing to C, this could add some time to run, > but I doubt that it could be important, as in general this is only done > once.
There still seems to be lacking any way to turn off the new XS code that is being run, in order to judge the performance impact. TEXINFO_XS_PARSER is now used for a lot of the new XS code, although it is not the parser. I tried commenting out all the new XS overrides (diff at the end of this mail) and this seemed to get the execution time back to what it was at the time of the last release, but I am not convinced it is functioning properly, as error messages are doubled. Before (typical time run reported): $ time ../tp/texi2any.pl ../../emacs-lispref-27.2/elisp.texi functions.texi:2390: warning: @inforef is obsolete errors.texi:226: warning: unexpected argument on @ignore line: The following seem to be unused now. real 0m5.664s user 0m5.409s sys 0m0.253s After (again, I chose typical results out of several): $ time ../tp/texi2any.pl ../../emacs-lispref-27.2/elisp.texi functions.texi:2390: warning: @inforef is obsolete errors.texi:226: warning: unexpected argument on @ignore line: The following seem to be unused now. functions.texi:2390: warning: @inforef is obsolete errors.texi:226: warning: unexpected argument on @ignore line: The following seem to be unused now. real 0m5.406s user 0m5.216s sys 0m0.188s These latter timings match more closely the timings of texi2any 7.1. Even though the error messages are doubled, the Info files appear to be correctly generated at a first glance. If I comment out a further block of code, I get rid of the duplicate errors: diff --git a/tp/texi2any.pl b/tp/texi2any.pl index 26cb16e2c0..0eaba5d07b 100755 --- a/tp/texi2any.pl +++ b/tp/texi2any.pl @@ -1634,13 +1634,13 @@ while(@input_files) { $document = Texinfo::Structuring::rebuild_document($document); - if ($with_XS) { - foreach my $error (@{$document->{'errors'}}) { - $registrar->add_formatted_message($error); - } - Texinfo::Structuring::clear_document_errors( - $document->document_descriptor()); - } +# if ($with_XS) { +# foreach my $error (@{$document->{'errors'}}) { +# $registrar->add_formatted_message($error); +# } +# Texinfo::Structuring::clear_document_errors( +# $document->document_descriptor()); +# } $error_count = handle_errors($registrar, $error_count, \@opened_files); (Test suite still doesn't pass with this though - probably a similar change is needed to tp/t/test_utils.pl but I haven't taken the time to investigate this.) (Simply commenting out $document = Texinfo::Structuring::rebuild_document($document); improved run times dramatically, but gave incorrect output without disabling the other XS code.) Can I please propose that it is made easy to disable all new XS code in texi2any, as I have done here, so we can avoid losing the performance of texi2any 7.1. I don't really care how it's done, as long as TEXINFO_XS and TEXINFO_XS_PARSER work as before (so TEXINFO_XS_PARSER should be limited to turning off the XS parser). It could be configured with a new variable TEXINFO_XS_MISC or TEXINFO_XS_NEW, or depend on TEXINFO_XS_CONVERT. It worries me that the new code is entwined with the program and hard to disable, and could lead to permanent slowdowns if the work doesn't proceed as hoped. It would seem much safer for the development of the program to separate it and be able to disable it cleanly. I expect that in the future, if XS converters actually do run, then these changes will be worth it, but they should only be turned on conditionally. diff --git a/tp/Texinfo/Common.pm b/tp/Texinfo/Common.pm index 01c5c118eb..e3f554b83f 100644 --- a/tp/Texinfo/Common.pm +++ b/tp/Texinfo/Common.pm @@ -83,32 +83,32 @@ sub import { if (!$module_loaded) { if (!defined $ENV{TEXINFO_XS_PARSER} or $ENV{TEXINFO_XS_PARSER} eq '1') { - Texinfo::XSLoader::override( - "Texinfo::Common::set_document_options", - "Texinfo::StructTransf::set_document_options"); - Texinfo::XSLoader::override( - "Texinfo::Common::copy_tree", - "Texinfo::StructTransf::copy_tree"); - Texinfo::XSLoader::override( - "Texinfo::Common::relate_index_entries_to_table_items_in_tree", - "Texinfo::StructTransf::relate_index_entries_to_table_items_in_tree" - ); - Texinfo::XSLoader::override( - "Texinfo::Common::move_index_entries_after_items_in_tree", - "Texinfo::StructTransf::move_index_entries_after_items_in_tree" - ); - Texinfo::XSLoader::override( - "Texinfo::Common::protect_colon_in_tree", - "Texinfo::StructTransf::protect_colon_in_tree" - ); - Texinfo::XSLoader::override( - "Texinfo::Common::protect_comma_in_tree", - "Texinfo::StructTransf::protect_comma_in_tree" - ); - Texinfo::XSLoader::override( - "Texinfo::Common::protect_node_after_label_in_tree", - "Texinfo::StructTransf::protect_node_after_label_in_tree" - ); +# Texinfo::XSLoader::override( +# "Texinfo::Common::set_document_options", +# "Texinfo::StructTransf::set_document_options"); +# Texinfo::XSLoader::override( +# "Texinfo::Common::copy_tree", +# "Texinfo::StructTransf::copy_tree"); +# Texinfo::XSLoader::override( +# "Texinfo::Common::relate_index_entries_to_table_items_in_tree", +# "Texinfo::StructTransf::relate_index_entries_to_table_items_in_tree" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Common::move_index_entries_after_items_in_tree", +# "Texinfo::StructTransf::move_index_entries_after_items_in_tree" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Common::protect_colon_in_tree", +# "Texinfo::StructTransf::protect_colon_in_tree" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Common::protect_comma_in_tree", +# "Texinfo::StructTransf::protect_comma_in_tree" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Common::protect_node_after_label_in_tree", +# "Texinfo::StructTransf::protect_node_after_label_in_tree" +# ); } $module_loaded = 1; } diff --git a/tp/Texinfo/Convert/HTML.pm b/tp/Texinfo/Convert/HTML.pm index 1099187716..0832c6c833 100644 --- a/tp/Texinfo/Convert/HTML.pm +++ b/tp/Texinfo/Convert/HTML.pm @@ -87,43 +87,43 @@ $VERSION = '7.1'; our $module_loaded = 0; sub import { if (!$module_loaded) { - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_default_format_protect_text", - "Texinfo::MiscXS::default_format_protect_text"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_entity_text", - "Texinfo::MiscXS::entity_text"); - - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_converter_initialize", - "Texinfo::Convert::ConvertXS::html_converter_initialize_sv"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_initialize_output_state", - "Texinfo::Convert::ConvertXS::html_initialize_output_state"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_sort_sortable_index_entries_by_letter", - "Texinfo::Convert::ConvertXS::sort_sortable_index_entries_by_letter"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_get_index_entries_sorted_by_letter", - "Texinfo::Convert::ConvertXS::get_index_entries_sorted_by_letter"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_prepare_conversion_units", - "Texinfo::Convert::ConvertXS::html_prepare_conversion_units"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_prepare_units_directions_files", - "Texinfo::Convert::ConvertXS::html_prepare_units_directions_files"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_prepare_output_units_global_targets", - "Texinfo::Convert::ConvertXS::html_prepare_output_units_global_targets"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_translate_names", - "Texinfo::Convert::ConvertXS::html_translate_names"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_html_convert_init", - "Texinfo::Convert::ConvertXS::html_convert_init"); - Texinfo::XSLoader::override( - "Texinfo::Convert::HTML::_XS_html_convert_convert", - "Texinfo::Convert::ConvertXS::html_convert_convert"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_default_format_protect_text", +# "Texinfo::MiscXS::default_format_protect_text"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_entity_text", +# "Texinfo::MiscXS::entity_text"); +# +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_converter_initialize", +# "Texinfo::Convert::ConvertXS::html_converter_initialize_sv"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_initialize_output_state", +# "Texinfo::Convert::ConvertXS::html_initialize_output_state"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_sort_sortable_index_entries_by_letter", +# "Texinfo::Convert::ConvertXS::sort_sortable_index_entries_by_letter"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_get_index_entries_sorted_by_letter", +# "Texinfo::Convert::ConvertXS::get_index_entries_sorted_by_letter"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_prepare_conversion_units", +# "Texinfo::Convert::ConvertXS::html_prepare_conversion_units"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_prepare_units_directions_files", +# "Texinfo::Convert::ConvertXS::html_prepare_units_directions_files"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_prepare_output_units_global_targets", +# "Texinfo::Convert::ConvertXS::html_prepare_output_units_global_targets"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_translate_names", +# "Texinfo::Convert::ConvertXS::html_translate_names"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_html_convert_init", +# "Texinfo::Convert::ConvertXS::html_convert_init"); +# Texinfo::XSLoader::override( +# "Texinfo::Convert::HTML::_XS_html_convert_convert", +# "Texinfo::Convert::ConvertXS::html_convert_convert"); $module_loaded = 1; } diff --git a/tp/Texinfo/Document.pm b/tp/Texinfo/Document.pm index 8b12698591..037e1b71c4 100644 --- a/tp/Texinfo/Document.pm +++ b/tp/Texinfo/Document.pm @@ -33,14 +33,14 @@ use Texinfo::StructTransf; our $module_loaded = 0; sub import { if (!$module_loaded) { - if (!defined $ENV{TEXINFO_XS_PARSER} - or $ENV{TEXINFO_XS_PARSER} eq '1') { - Texinfo::XSLoader::override( - "Texinfo::Document::remove_document", - # TODO add a Document XS .xs file and move to that file? - "Texinfo::StructTransf::remove_document" - ); - } + # if (!defined $ENV{TEXINFO_XS_PARSER} + # or $ENV{TEXINFO_XS_PARSER} eq '1') { + # Texinfo::XSLoader::override( + # "Texinfo::Document::remove_document", + # # TODO add a Document XS .xs file and move to that file? + # "Texinfo::StructTransf::remove_document" + # ); + # } $module_loaded = 1; } # The usual import method diff --git a/tp/Texinfo/Structuring.pm b/tp/Texinfo/Structuring.pm index 2b5b4f5a86..6af8e760d2 100644 --- a/tp/Texinfo/Structuring.pm +++ b/tp/Texinfo/Structuring.pm @@ -87,66 +87,66 @@ sub import { if (!$module_loaded) { if (!defined $ENV{TEXINFO_XS_PARSER} or $ENV{TEXINFO_XS_PARSER} ne '0') { - Texinfo::XSLoader::override( - "Texinfo::Structuring::rebuild_document", - "Texinfo::StructTransf::rebuild_document"); - Texinfo::XSLoader::override( - "Texinfo::Structuring::rebuild_tree", - "Texinfo::StructTransf::rebuild_tree"); - Texinfo::XSLoader::override( - "Texinfo::Structuring::clear_document_errors", - "Texinfo::StructTransf::clear_document_errors"); - Texinfo::XSLoader::override( - "Texinfo::Structuring::associate_internal_references", - "Texinfo::StructTransf::associate_internal_references" - ); - Texinfo::XSLoader::override( - "Texinfo::Structuring::sectioning_structure", - "Texinfo::StructTransf::sectioning_structure" - ); - Texinfo::XSLoader::override( - "Texinfo::Structuring::warn_non_empty_parts", - "Texinfo::StructTransf::warn_non_empty_parts" - ); - Texinfo::XSLoader::override( - "Texinfo::Structuring::nodes_tree", - "Texinfo::StructTransf::nodes_tree" - ); - Texinfo::XSLoader::override( - "Texinfo::Structuring::set_menus_node_directions", - "Texinfo::StructTransf::set_menus_node_directions" - ); - Texinfo::XSLoader::override( - "Texinfo::Structuring::complete_node_tree_with_menus", - "Texinfo::StructTransf::complete_node_tree_with_menus" - ); - Texinfo::XSLoader::override( - "Texinfo::Structuring::check_nodes_are_referenced", - "Texinfo::StructTransf::check_nodes_are_referenced" - ); - Texinfo::XSLoader::override( - "Texinfo::Structuring::number_floats", - "Texinfo::StructTransf::number_floats" - ); - Texinfo::XSLoader::override( - "Texinfo::Structuring::rebuild_output_units", - "Texinfo::StructTransf::rebuild_output_units"); - Texinfo::XSLoader::override( - "Texinfo::Structuring::_XS_unsplit", - "Texinfo::StructTransf::unsplit"); - # Not useful for HTML as functions, as the calling functions are - # already overriden - # Could be readded when other converters than HTML are done in C - #Texinfo::XSLoader::override( - # "Texinfo::Structuring::split_by_node", - # "Texinfo::StructTransf::split_by_node"); - #Texinfo::XSLoader::override( - # "Texinfo::Structuring::split_by_section", - # "Texinfo::StructTransf::split_by_section"); - #Texinfo::XSLoader::override( - # "Texinfo::Structuring::split_pages", - # "Texinfo::StructTransf::split_pages" - #); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::rebuild_document", +# "Texinfo::StructTransf::rebuild_document"); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::rebuild_tree", +# "Texinfo::StructTransf::rebuild_tree"); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::clear_document_errors", +# "Texinfo::StructTransf::clear_document_errors"); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::associate_internal_references", +# "Texinfo::StructTransf::associate_internal_references" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::sectioning_structure", +# "Texinfo::StructTransf::sectioning_structure" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::warn_non_empty_parts", +# "Texinfo::StructTransf::warn_non_empty_parts" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::nodes_tree", +# "Texinfo::StructTransf::nodes_tree" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::set_menus_node_directions", +# "Texinfo::StructTransf::set_menus_node_directions" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::complete_node_tree_with_menus", +# "Texinfo::StructTransf::complete_node_tree_with_menus" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::check_nodes_are_referenced", +# "Texinfo::StructTransf::check_nodes_are_referenced" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::number_floats", +# "Texinfo::StructTransf::number_floats" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::rebuild_output_units", +# "Texinfo::StructTransf::rebuild_output_units"); +# Texinfo::XSLoader::override( +# "Texinfo::Structuring::_XS_unsplit", +# "Texinfo::StructTransf::unsplit"); +# # Not useful for HTML as functions, as the calling functions are +# # already overriden +# # Could be readded when other converters than HTML are done in C +# #Texinfo::XSLoader::override( +# # "Texinfo::Structuring::split_by_node", +# # "Texinfo::StructTransf::split_by_node"); +# #Texinfo::XSLoader::override( +# # "Texinfo::Structuring::split_by_section", +# # "Texinfo::StructTransf::split_by_section"); +# #Texinfo::XSLoader::override( +# # "Texinfo::Structuring::split_pages", +# # "Texinfo::StructTransf::split_pages" +# #); } $module_loaded = 1; } diff --git a/tp/Texinfo/Transformations.pm b/tp/Texinfo/Transformations.pm index 836e1b4aae..0a98e2016b 100644 --- a/tp/Texinfo/Transformations.pm +++ b/tp/Texinfo/Transformations.pm @@ -54,41 +54,41 @@ $VERSION = '7.1'; our $module_loaded = 0; sub import { if (!$module_loaded) { - if (!defined $ENV{TEXINFO_XS_PARSER} - or $ENV{TEXINFO_XS_PARSER} eq '1') { - Texinfo::XSLoader::override( - "Texinfo::Transformations::fill_gaps_in_sectioning", - "Texinfo::StructTransf::fill_gaps_in_sectioning" - ); - Texinfo::XSLoader::override( - "Texinfo::Transformations::reference_to_arg_in_tree", - "Texinfo::StructTransf::reference_to_arg_in_tree" - ); - Texinfo::XSLoader::override( - "Texinfo::Transformations::complete_tree_nodes_menus", - "Texinfo::StructTransf::complete_tree_nodes_menus" - ); - Texinfo::XSLoader::override( - "Texinfo::Transformations::complete_tree_nodes_missing_menu", - "Texinfo::StructTransf::complete_tree_nodes_missing_menu" - ); - Texinfo::XSLoader::override( - "Texinfo::Transformations::regenerate_master_menu", - "Texinfo::StructTransf::regenerate_master_menu" - ); - Texinfo::XSLoader::override( - "Texinfo::Transformations::insert_nodes_for_sectioning_commands", - "Texinfo::StructTransf::insert_nodes_for_sectioning_commands" - ); - Texinfo::XSLoader::override( - "Texinfo::Transformations::protect_hashchar_at_line_beginning", - "Texinfo::StructTransf::protect_hashchar_at_line_beginning" - ); - Texinfo::XSLoader::override( - "Texinfo::Transformations::protect_first_parenthesis_in_targets", - "Texinfo::StructTransf::protect_first_parenthesis_in_targets" - ); - } +# if (!defined $ENV{TEXINFO_XS_PARSER} +# or $ENV{TEXINFO_XS_PARSER} eq '1') { +# Texinfo::XSLoader::override( +# "Texinfo::Transformations::fill_gaps_in_sectioning", +# "Texinfo::StructTransf::fill_gaps_in_sectioning" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Transformations::reference_to_arg_in_tree", +# "Texinfo::StructTransf::reference_to_arg_in_tree" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Transformations::complete_tree_nodes_menus", +# "Texinfo::StructTransf::complete_tree_nodes_menus" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Transformations::complete_tree_nodes_missing_menu", +# "Texinfo::StructTransf::complete_tree_nodes_missing_menu" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Transformations::regenerate_master_menu", +# "Texinfo::StructTransf::regenerate_master_menu" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Transformations::insert_nodes_for_sectioning_commands", +# "Texinfo::StructTransf::insert_nodes_for_sectioning_commands" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Transformations::protect_hashchar_at_line_beginning", +# "Texinfo::StructTransf::protect_hashchar_at_line_beginning" +# ); +# Texinfo::XSLoader::override( +# "Texinfo::Transformations::protect_first_parenthesis_in_targets", +# "Texinfo::StructTransf::protect_first_parenthesis_in_targets" +# ); +# } $module_loaded = 1; } # The usual import method diff --git a/tp/Texinfo/TranslationsNonXS.pm b/tp/Texinfo/TranslationsNonXS.pm index 5e9af44afe..283bfb8d59 100644 --- a/tp/Texinfo/TranslationsNonXS.pm +++ b/tp/Texinfo/TranslationsNonXS.pm @@ -47,12 +47,12 @@ Locale::Messages->select_package ('gettext_pp'); our $module_loaded = 0; sub import { if (!$module_loaded) { - # the first one is defined in this here perl file. - # the second one is defined in the Translations.xs file. - Texinfo::XSLoader::override( - "Texinfo::Translations::_configure_XS", - "Texinfo::Translations::configure_XS" - ); +# # the first one is defined in this here perl file. +# # the second one is defined in the Translations.xs file. +# Texinfo::XSLoader::override( +# "Texinfo::Translations::_configure_XS", +# "Texinfo::Translations::configure_XS" +# ); $module_loaded = 1; } # The usual import method