Skip to content

Value compaction seems to do a lot of unnecessary work #653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daenney opened this issue Apr 17, 2025 · 0 comments
Open

Value compaction seems to do a lot of unnecessary work #653

daenney opened this issue Apr 17, 2025 · 0 comments

Comments

@daenney
Copy link

daenney commented Apr 17, 2025

While I was implementing value compaction as per https://www.w3.org/TR/json-ld11-api/#value-compaction there seems to be some unnecessary work being performed.

Steps 2 and 3 are unnecessary, the inverse context is never used during value compaction. I assume this is a left-over from a previous draft of the spec?

Compact value is only ever called once during Compaction, in step 7:

If element has an @value or @id entry and the result of using the Value Compaction algorithm, passing active context, active property, and element as value is a scalar, or the term definition for active property has a type mapping of @json, return that result.

With the exception of the @json entry, the result from value compaction is always discarded if it's not a scalar. This would suggest that every step in Value Compaction where we return anything other than a scalar is not necessary. For a value that's @json, we don't want to perform IRI compaction of the keys as far as I understand it, which would also make step 11 in value compaction unnecessary. I might be wrong about this part. Since we only ever care for the scalar return, creating the copy of result in step 1 isn't necessary either.

It seems like we can simplify things by:

  • In Compaction step 7:
    • First check if the type mapping is @json and return value without going through Value Compaction.
    • Then do the value compaction and return it if it's not null.
  • Update the Value Compaction as follows:
    • Drop 1), 2), 3).
    • In step 8) keep the branch but do nothing.
    • Drop 11).
    • Update 12) to return null

These changes seem to work in my own processor without it causing anything in the compaction test suite to fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

1 participant