[torch.compile] split shared compilation hash helpers

[vnadathur]

Motivation

PR: #26468 removed envs.compute_hash which was being used by compilation_config_hash_factors, which decorators.py relies on for aot cache keys.

Both _compute_code_hash and compilation_config_hash_factors exist to keep aot and jit hashing consistent. backends.py had a duplicate of their logic

The Fix

This pr creates a shared helper instead of inlining them and updates the helper itself to call the new envs.compile_factors().

Thanks for pointing this out @zhxchen17

Test

@zhxchen17 does this fix the issue with pytorch nightly?

cc @ProExpertProg @WorldExplored

[gemini-code-assist]

Code Review

This pull request does a good job of refactoring the compilation hashing logic into shared helpers, which improves code reuse and maintainability. However, I've found a critical issue where the hashing algorithm for code hashing was changed from sha256 to md5, which could lead to cache inconsistencies and collisions. I've also identified a performance issue due to a redundant function call. Please see my detailed comments for suggestions on how to address these points.

[ProExpertProg]

Can we instead refactor the decorator to use the new & cleaner approach instead of restoring the older code?

[vnadathur]

@zhxchen17 i tried to hit ur concern here:

In other words, we don't have the same hashing algorithm between VLLM_USE_AOT_COMPILE=1 and VLLM_USE_AOT_COMPILE=0. I wonder does anyone think that will be a problem?

[zhxchen17]

Thanks for the fix! I think this looks ok to me as well.

[zhxchen17]

@zhxchen17 i tried to hit ur concern here:

In other words, we don't have the same hashing algorithm between VLLM_USE_AOT_COMPILE=1 and VLLM_USE_AOT_COMPILE=0. I wonder does anyone think that will be a problem?

@vnadathur Deleted my original comment there since I think that's not accurate.

@ProExpertProg I like this fix slightly better because it shares the code for code hashing. I think in the short term it's okay to remove compilation_config_hash_factors tho. I wonder whether it will be a problem if in the future we decide to introduce a new common source of hashing (in addition to env+compiler config) that only got added to one path but not the other one. If we think it's unlikely, I think it's fine to have small duplication.

[ProExpertProg]

I think if we're cleaning this up, let's really clean it up. I would prefer to land #29058 for now. I agree it would be good to keep the two cache hashes in sync but they are already different and the only common part is hash of config and envs, both of which come from the same member methods on the config and envs already.

[ProExpertProg]

This looks fine as well, @zou3519 @drisspg @zhxchen17 can you check this works for you, happy to merge as is and improve structure later

[ProExpertProg]

Let's merge #29058 for now. After that we can properly restructure the config, including pulling out the compile cache factors so they can be stored with other cache artifacts

[ProExpertProg]

Let's just do this in #29117, can we close this?