[5.x] Augmentation performance improvements #9636
Merged
Conversation
This file contains 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
Positive improvements across a variety of different situations, particularly the `group_by` modifier
JohnathonKoster
changed the title
* Allow for implementors to return `null`, which disables the bulk process for that item * Entry now takes data keys into consideration to prevent "locking" an entry into something if it has custom data
37 tasks
# Conflicts: # src/Entries/Entry.php
…some just because ... - Dedicated AbstractAugmented 'wrap' methods, to avoid passing around hard to follow booleans. - Avoid a hard to follow boolean in InvokableValue - Make InvokableValue easier to follow both paths in general. - TransientValue and DeferredValue now pass their references when calling shallow(), which were missing, and was the reason for needing to check for nulls. - Typehints and visibility
…ng when its an actual Value instance.
… ... If you pass a closure as the raw value, it'll lazily evaluate them when resolving.
materialize() just called resolve() and returned a new Value instance. Now that the subclasses are gone, there's no need to new up a separate Value instance. It can just return itself. With that gone, we might as well just call resolve since materialize would just be an alias.
jasonvarga
changed the title
- Add static factory methods to BulkAugmentor. - Rename BulkAugmentor augmented() to toArray(). Seemed more inline with what it did. Looks like it makes sense when compared to the toDeferredAugmentedArray in the "else" where its used in RuntimeValues. - Reduce visibility where possible
Since dd(), dump(), etc can have variable arguments, avoid making our own Dumper methods. Just use the class to resolve the values and pass them into the original functions. Rename resolveValues to resolve. Simplify map. Remove unnecessary guard.
1 task
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.
This PR is an extension of #9574, and reworks some more internal augmentation behaviors. The changes in this PR come together to allow us to reuse fields arrays and augmentation keys as much as possible in a new
BulkAugmentor. This new class will work to reduce the overhead of resolving this information as much as possible. For example, if you want to augment 1,000 entries and they can all safely reuse the same fields/keys, theBulkAugmentorwill only do those calls once. This PR only focuses on pages/entries for now.Valueobjectsgetdirectly, they shouldn't need to care about materializationInvokableValueandDeferredValueeven more lazy 🥳Rework collection tag to use transient valueswill just handle this Antlers-side to cause less chaos