Remove deduplication of Request after source tracking#2075
Merged
Conversation
After source tracking, adding a Request to a HashSet is ineffective because it contains a unique location for each request. As a side-effect, removing the deduplication means URLs will be sent to the stream in file order, which is nice for consistency.
Member
|
Thank you. This reduces unnecessary complexity for a bit. Basically this duplication was useless as of now (it might have been useful in the past) because a |
| /// request errors are not deduplicated. | ||
| /// Create requests out of the collected URLs. Returns a vector of valid URLs | ||
| /// and errors. URLs are not deduplicated because repeated URLs may occur at | ||
| /// different source locations. |
Member
There was a problem hiding this comment.
Maybe it would be worth mentioning that we don't do deduplication as it happens on a much lower level? (in host.rs) Also Returns a vector of valid URLs and errors is a bit confusing.
We could simplify it maybe something in the direction of:
Suggested change
| /// different source locations. | |
| /// Create requests out of the collected URLs. | |
| /// [`Request`]s are not deduplicated. | |
| /// Caching and deduplication happens on a lower level. |
+/// Requests are not deduplicated because repeated URLs may occur at different +/// source locations. Caching and deduplication happens elsewhere (e.g., in the +/// per-host HostCache and the top-level persistent cache).
+/// Create requests out of the collected URLs. Maps each [`RawUri`] to a parsed +/// [`Request`] or a [`RequestError`], according to the given options.
Member
Author
|
Let me know what you think after those commits. |
Member
|
Looks great, thank you! |
Merged
Merged
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
2 participants
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.
After source tracking, adding a Request to a HashSet is ineffective because it contains a unique location for each request.
As a side-effect, removing the deduplication means URLs will be sent to the stream in file order, which is nice for consistency.