Skip to content

Speed up of compare_values and has_value methods#101

Open
RinkeHoekstra wants to merge 3 commits intodigitalbazaar:linter-jsonldfrom
RinkeHoekstra:comparison-speed
Open

Speed up of compare_values and has_value methods#101
RinkeHoekstra wants to merge 3 commits intodigitalbazaar:linter-jsonldfrom
RinkeHoekstra:comparison-speed

Conversation

@RinkeHoekstra
Copy link

I noticed very slow performance on the to_rdf procedure for a JSON-LD file with several tens of thousands of typed object values for a single property.

Running cProfiles, it turned out that there was an inordinate amount of type spent in the methods compare_values and has_value.

This pull request introduces the following changes:

  • Re-implemented compare_values with exception handling rather than if-then statements. Also changed the ordering and removed the boolean comparison between primitive values (It may need to return, but I couldn't understand the reason behind it)
  • Re-implemented the has_value method (which called compare_values so very frequently) to perform checks only once, and only compare values of the same type.

There's also a question on the way the has_value is implemented: it seems that if the value parameter that's passed is an array, it is completely ignored. Is that the correct behavior?

@davidlehn
Copy link
Member

Thanks, I'll take a look when I get a chance. May need to finish getting the test suite up-to-date so we can check if the changes are ok.

Can you give a brief example of the form of data that was slow? Just one or two properties is fine, not 10000+ :-) I've started setting up a benchmarking system and issues like this are useful target for auto-generating test data inputs.

@RinkeHoekstra
Copy link
Author

RinkeHoekstra commented Oct 22, 2020

I'd like to bump this as it fixes major issues we're facing internally, with slow framing on large JSON-LD files.

I will try to generate some artificial data (large amount of data, few properties).

@dvsrepo
Copy link

dvsrepo commented Dec 3, 2020

Hi! Any plans to include this? We are also facing issues with framing large JSON-LD files.

@mielvds mielvds added this to the v3.0.0 (JSON-LD 1.1) milestone Jan 8, 2026
@mielvds
Copy link
Collaborator

mielvds commented Feb 9, 2026

@RinkeHoekstra I know it's been a while, but now that there are spec tests, this performance fix makes a Flattening and Transform test fail: https://github.com/digitalbazaar/pyld/actions/runs/21819124584/job/62947739897?pr=101

@mielvds mielvds changed the base branch from master to linter-jsonld February 27, 2026 09:42
@mielvds
Copy link
Collaborator

mielvds commented Feb 27, 2026

I have fixed the bugs in the perfomance version; the test-suite passes now. Not all performance gains have been kept, but there is still significant speedup.

@mielvds mielvds force-pushed the comparison-speed branch from 7f93c71 to 181e4de Compare March 2, 2026 09:32
@mielvds
Copy link
Collaborator

mielvds commented Mar 2, 2026

@dvsrepo if after 5 years, you're still in the position of benchmarking this, please do :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants