[Optimization] Avoid repeated model architecture conversion for pooling models

[DarkLight1337]

Purpose

When running multi-modal pooling models, I found that almost 3% of the time is taken by the get_model_architecture call inside create_processor. Upon inspection, get_model_architecture converts the model class into a pooling model each time it is called which is quite expensive (since it occurs for every single request), so I have decided to cache its output.

cc @maxdebayser @noooop

Test Plan

Test Result

---

Essential Elements of an Effective PR Description Checklist

• The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
• The test plan, such as providing test command.
• The test results, such as pasting the results comparison before and after, or e2e results
• (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
• (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

[gemini-code-assist]

Code Review

This pull request introduces a cache for get_model_architecture to avoid repeated expensive conversions, which is a good optimization. My main feedback is regarding thread safety. The global cache is modified without a lock, which can lead to race conditions and redundant computations in a multi-threaded environment. I've suggested adding a lock using a double-checked locking pattern to make the caching mechanism thread-safe and robust.

[maxdebayser]

Yeah, I noticed too that it was being called repeatedly. LGTM, just a quick question though, would this work with functools cache?

[DarkLight1337]

No, since ModelConfig itself is not hashable directly

[maxdebayser]

But if ModelConfig already has a compute_hash() function it should be possible to implement __hash__ using it.

[DarkLight1337]

But if ModelConfig already has a compute_hash() function it should be possible to implement __hash__ using it.

That is true, but compute_hash only takes into account the fields that affect CUDA graph compilation. I don't want to implement __hash__ using this as that would violate the hash-equals contract.

[DarkLight1337]

Hmm... looks like this PR uncovered some hidden problems about EAGLE config validity

[DarkLight1337]

Any idea? @wwl2755 @WoosukKwon

[wwl2755]

I think it is because EAGLEConfig() won't expect model=None is their use case. The use case is something like:

eagle_config = EAGLEConfig(
                            self.draft_model_config.hf_config, # model
                            method=self.method,
                            model_type="eagle")

So it makes a problem when compute_hash() try to use the default initialization via EAGLEConfig().

An easy fix is to delete the assertion and make a dummy kwargs["architectures"] = [] to cooperate the compute_hash(). IMO, it won't affect the normal behavior as it will always pass something in model parameter. But not 100% sure whether it will break any potential consensus other where. Leave it to @WoosukKwon for the decision call.

[DarkLight1337]

Hmm actually the test runs successfully on its own. Maybe the config is somehow modified in place by other tests, let me take a look Nvm I was running the test on the wrong branch 😅

[DarkLight1337]

Ok I just realized that to_json_string only returns diffs by default which causes EAGLEConfig to be instantiated without model. Let's see if use_diff=False can solve the problem

[wwl2755]

Let's see if use_diff=False can solve the problem

Great catch! Solve the problem once for all.