Open
Conversation
Performance analysis shows that Symbol::all_fields() is the most expensive function call during record validation. In order to avoid redundant recomputation when multiple records reference the same model in a single XML file, we cache results per model name. Other changes for performance or code improvements: - Deduplicate get_main_symbols call in validate_record - Replace unnecessary diagnostic.clone() with moves - Comment out no-op add_model_dependencies call (XmlFileSymbol not handled), and removed model_dependencies from method parameters - Comment out mandatory_fields computation, which is not read anywhere - Store module on XmlValidator struct, remove from all method parameters - Rename `validate_xml_id` to `validate_data` for clarity (as we are not validating the XML ID itself, but the whole data entry)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Performance analysis shows that Symbol::all_fields() is the most expensive function call during record validation. In order to avoid redundant recomputation when multiple records reference the same model in a single XML file, we cache results per model name.
Other changes for performance or code improvements:
validate_xml_idtovalidate_datafor clarity (as we are not validating the XML ID itself, but the whole data entry)Measured improvements using tracing-timing (as per alpha-imp_perf_xml_validation-with-instrumentation-rcdl), running odoo_ls_server in --parse mode with community + enterprise 19.2:
Before
Timing histogram span="xml_validation" event="close" count=6687 p50_ms=0.147455 p90_ms=0.962559 p99_ms=8.126463 max_ms=149.946367 total_ms=4089.7898720000026
After
Timing histogram span="xml_validation" event="close" count=6687 p50_ms=0.112127 p90_ms=0.288767 p99_ms=0.724991 max_ms=5.963775 total_ms=968.1314160000005