Skip to content
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

Remove line from crc32 to be able to track functions in changed files #21

Closed
wprigollopes opened this issue Jul 8, 2022 · 2 comments
Closed

Comments

@wprigollopes
Copy link

return crc32($this->file->getReferencePath()."\n".$this->line."\n".$this->functionName."\n".implode(',', $this->arguments));

When we save the logger, we use the tombstone hash to save the file log. As we are using the current line call, if we modify the file for any purpose, the line could change so, when we run a new tombstone-analyzer routine we will have one deleted tombstone call and a new one dead call.

So, for this situation, can we consider removing the line from the tombstone hash?

@scheb
Copy link
Owner

scheb commented Jul 8, 2022

As we are using the current line call, if we modify the file for any purpose, the line could change so, when we run a new tombstone-analyzer routine we will have one deleted tombstone call and a new one dead call.

That's actually not true. The hash value is only used to deduplicate repeating calls from the same tombstone. That doesn't change anything about weather a tombstone is dead, undead or deleted.

Matching logged tombstone calls to tombstones in the codebase is based on two strategies:

  • Location of the tombstone (file + line + inscription)
  • Context of the tombstone (surrounding method/function + inscription)

If one of those matches, the logged call is assigned to the tombstone. So you can move a tombstone around, as long as one of those criteria stays the same the analyzer would still be able to match it.

Though, if you heavily refactor your codebase (change method names and change code to shift tombstones around), then it can in fact happen, that previously logged tombstone calls are no longer be matched to that tombstone in the code. In that case, the previously logged tombstone calls would show up as deleted and you'd get new tombstones showing up in the report. Once that code is executed and new tombstone calls are being sampled, you should quickly get a refreshed clear picture, which ones are dead and which ones aren't. But for that you have to give the system some time to collect new samples after a heavy refactoring.

If this way of matching is not good or stable enough for you, the idea for custom matching strategies is here, but it hasn't gained much interest yet.

@wprigollopes
Copy link
Author

Those two matches look like a good strategy. The only number that could be not exact is the deleted but just in extreme cases. Also, I'm considering that the analyzer needs to be run periodically to have a good summary of the evolution of dead code results and to maintain effective statistics of the code.

Thanks for the clarification.

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

No branches or pull requests

2 participants