-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Multiple execution of checksum calculation in Specification #7854
Comments
what is the stacktrace? which methods calls this many times? |
you can put |
I am very sorry.... I found that there is a plugin to compare the checksum in my project, and it will cause the It is not a bug, but I still hope CP could use
It does not perform many computations, include the subspecs. |
We want to move away from You can cache the result without an |
Alternatively you can improve the plugin so it doesn't invoke this many times. |
I can cache the checksum in my plugin. But I could not void to skip the first caller. It meant that I must get the checksum at first then cache it. It is still called once. On the another hand, the subspec should get the checksum from the root spec, right? |
it does not matter. It seems its reading the file for the checksum see checksum = Digest::SHA1.hexdigest(File.read(defined_in_file)) # see `defined_in_file` So whether its called from subspec or root spec its the same file. |
@Whirlwind you cannot avoid the first caller right? Even if its cached the first time will have to do the expensive part once. Do you still want this issue open? |
Right,it is the same file. But it will cause many times read the file and get the checksum. |
Could you tell the reason? |
you are right that we could optimize it for subspec to delegate to the root value and cache that (basically your change). I think its best to make a PR for this. |
If you agree with me, I will make a PR based my change. |
Maybe you have a better way to cache it. |
You can do your change without an |
def checksum
@checksum ||= begin
if root?
unless defined_in_file.nil?
require 'digest'
checksum = Digest::SHA1.hexdigest(File.read(defined_in_file))
checksum = checksum.encode('UTF-8') if checksum.respond_to?(:encode)
checksum
end
else
root.checksum
end
end
end Like this? |
yes! I think that should do it. |
OK, I will make a PR later. |
Should the checksum in |
Thanks for the PR. |
It only gets invoked twice...but sure I dont think its harmful. |
When I check the code from Specification, I found the
checksum
will be called many times. I try to add some code to debug it:get the output:
I think we could cache the checksum by the path.
The text was updated successfully, but these errors were encountered: