-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Hash input file contents for imageproc output filenames #2886
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
base: next
Are you sure you want to change the base?
Conversation
|
Hm... this is vulnerable to the input contents changing between enqueueing and image processing. I suppose the queue would have to hold the input contents instead of the input path? |
|
I think that makes sense |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea but the performance changes seem to be a net negative even when you take into account regenerating the image if we change the filename
| ) -> Result<String> { | ||
| let mut hasher = DefaultHasher::new(); | ||
| hasher.write(input_src.as_ref()); | ||
| hasher.write( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does that change perf significantly? I can imagine reading a whole site of images is going to be much more computationally expensive than filenames
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it doesn't make sense to read the file multiple times, once for the filename and once to actually operate on it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I've been thinking about how to approach this. We'd essentially want to read each input once (at most), and for each operation to hold a reference or key to the in-memory contents of the input for processing.
I'll push something for that if I figure out a good solution.
3801386 to
35110a5
Compare
35110a5 to
a7b7acb
Compare
It doesn't seem quite right to me that
resize_image's output filename hash reads the path of the input file; shouldn't it read the contents instead? That way:(Also,
GetImageMetadata::callcan just use the full path to the file as its cache key, so the return signature ofsearch_for_filecan be simplified.)Since #2862/#2872 have changed the hash behavior on
next, I figure this is maybe a good time to consider this, too, as it also affects the imageproc hash behavior.