tools-python icon indicating copy to clipboard operation
tools-python copied to clipboard

Validation performance extremely poor

Open billie-alsup opened this issue 6 months ago • 5 comments

I am finding that validation is extremely slow (taking 3+ hours to validate a document that took a fraction of the time to create). A sample cProfile run shows

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
...
        1    0.000    0.000 15303.455 15303.455 document_validator.py:19(validate_full_spdx_document)
   288826   87.452    0.000 15125.126    0.052 spdx_id_validators.py:46(validate_spdx_id)
        1    0.623    0.623 15122.608 15122.608 relationship_validator.py:12(validate_relationships)
   107391    3.714    0.000 15121.905    0.141 relationship_validator.py:22(validate_relationship)
   213108  589.004    0.003 15032.507    0.071 spdx_id_validators.py:25(is_spdx_id_present_in_document)
   213109   88.149    0.000 14443.590    0.068 spdx_id_validators.py:31(get_list_of_all_spdx_ids)
   213110 3420.272    0.016 14003.702    0.066 document_utils.py:11(get_contained_spdx_element_ids)
17340975234 4410.662    0.000 11361.884    0.000 dataclass_with_properties.py:46(get_field)
17343871659 6951.929    0.000 6951.929    0.000 {built-in method builtins.getattr}

The implementation seems to always use sequential scanning over arrays when performing validation, when it might more sense to create an index for a particular array, and use that during validation. Is there any plan to improve performance, at least the validation performance? There the necessary indices for speed improvement could be created dynamically and used during validation. Having such indices available while building the document would be fine as well, although would put some restrictions on how the document is updated. I realize that the arrays are providing a strict ordering that should probably be maintained, so it is not as simple as replacing the arrays with a dictionary.

In the example profile, the function get_list_of_all_spdx_ids is called every time is_spdx_id_present_in_document is invoked. This is a prime example where generating an index at the start of validation and using it during validation would be a win. Perhaps we could add some private members to Document class as the index in this case?

def is_spdx_id_present_in_files(spdx_id: str, files: List[File]) -> bool:
    return spdx_id in [file.spdx_id for file in files]


def is_spdx_id_present_in_document(spdx_id: str, document: Document) -> bool:
    all_spdx_ids_in_document: List[str] = get_list_of_all_spdx_ids(document)

    return spdx_id in all_spdx_ids_in_document


def get_list_of_all_spdx_ids(document: Document) -> List[str]:
    all_spdx_ids_in_document: List[str] = [document.creation_info.spdx_id]
    all_spdx_ids_in_document.extend(get_contained_spdx_element_ids(document))

    return all_spdx_ids_in_document


def is_external_doc_ref_present_in_document(external_ref_id: str, document: Document) -> bool:
    all_external_doc_ref_ids_in_document = [
        external_doc_ref.document_ref_id for external_doc_ref in document.creation_info.external_document_refs
    ]

    return external_ref_id in all_external_doc_ref_ids_in_document

billie-alsup avatar Aug 05 '24 21:08 billie-alsup