1994491 - Link Preview don't reset engine/numContext for llama-cpp

Status: RESOLVED WONTFIX

(Core :: Machine Learning: On Device, defect, P2)

Description

[Aristide Tossou]

This means we for best-engine == llama.cpp, we don't reset the numContext (ideally set the just an initial) values. By not resetting the numContext, the engine does not need to be shutdown.

This is possible because the llama.cpp backend dynamically increase numContext as needed.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:tarek, could you have a look please?

For more information, please visit BugBot documentation.

Comment 2

[Greg Tatum [:gregtatum]]

The description here is confusing, but here is what Claude says:

  What the Bug is About

  The root problem: In browser/components/genai/LinkPreviewModel.sys.mjs:410-413, numContext is
  dynamically calculated per-request based on article length:

  const numContext =
    Math.ceil(
      (processedInput.length + systemPrompt.length) * CONTEXT_SIZE_MULTIPLIER
    ) + nPredict;

  This value is then passed as a pipeline option when creating/reusing the engine (line 427-428).

  Engine reuse logic: In toolkit/components/ml/actors/MLEngineParent.sys.mjs:289-308, engines are
  reused only if pipelineOptions.equals(other) returns true. If options differ — including numContext
   — the old engine is terminated and replaced.

  So for every article of a different length, the engine gets shut down and restarted with a new
  numContext. That's expensive.

  The llama.cpp distinction: The native llama.cpp backend (LlamaCppPipeline.mjs:169-172) passes nCtx
  to LlamaRunner which can grow context dynamically as needed. Unlike WASM-based wllama, you don't
  need to pre-allocate the exact right size — it'll expand. So restarting the engine to change
  numContext is unnecessary waste for llama.cpp.

  What the Fix Would Be

  For backend === "llama.cpp", instead of recalculating and setting numContext each call (which
  forces engine replacement), you'd either:
  - Set a fixed initial numContext (e.g., the default 1024 or some reasonable minimum) and let
  llama.cpp grow dynamically, OR
  - Exclude numContext from the PipelineOptions.equals() comparison when the backend is llama.cpp

  This way the pipeline options stay stable across calls → engine gets reused → no shutdown/restart
  overhead between articles.

  The bug has real merit. The writing is rough, but the underlying observation is correct: the
  dynamic numContext calculation causes unnecessary engine churn specifically for llama.cpp, which
  doesn't need it.

I wonder if this is related to: Bug 2015887

Comment 3

[Valentin Pollet]

I took a deeper look at the LlamaRunner/LlamaBackend implementation and I tend to agree with Claude's analysis. Here are my observations:

The Inference Process was designed with engine reuse in mind, with a time-to-live and a way to keep backends warm between calls. llama.cpp is even friendly to this, because it cleanly separates things:

• llama_model is a thread-safe abstraction designed to be shared. It owns a loaded model, ready to be re-used for inference.
• llama_context is an abstraction designed to be used from a single thread for inference, taking a llama_model and parameters like the size of the context window.

LlamaBackend conflates both, as it owns a llama_model and a llama_context (https://searchfox.org/firefox-main/source/toolkit/components/ml/backends/llama/LlamaBackend.h#132-137). The inference process decides if an existing, warm backend can be reused for a subsequent request by comparing PipelineOptions. For llama.cpp, those options contain the size of the context window, meaning that changing the context window size invalidates the engine cache, preventing any reuse.

For Link Preview specifically, I validated Claude's claim. Every call to Link Preview derives the size of the context window from the length of the input using a very heuristic computation of the size of the input in numbers of tokens. (https://searchfox.org/firefox-main/source/browser/components/genai/LinkPreviewModel.sys.mjs#404-413) That means no re-use is possible. Luckily, we are not reloading hundreds of megabytes by reading disk at each call, because llama.cpp has an option to use mmap to mitigate the cost of reloading from disk at each call to llama_load_model_from_file. It does not make the reinitialization free and measurements are needed here to convince me that this is a non-issue performance-wise.

An additional thing to keep in mind to make engine reuse possible is that Link Preview forces engine tear-down when generation is over: LinkPreviewModel explicitly tears down the engine when generation is done (https://searchfox.org/firefox-main/source/browser/components/genai/LinkPreviewModel.sys.mjs#479)

Another additional thing to keep in mind is that right now Link Preview guesses the context window size to instantiate the engine as it requires it. The backend then checks this value against the actual size that it gets using llama.cpp directly. In case the estimate was wrong, the backend rebuilds the llama_context entirely to fit. (https://searchfox.org/firefox-main/source/toolkit/components/ml/backends/llama/LlamaBackend.cpp#502-523). This feels quite wasteful, as in some cases we end up rebuilding the native llama.cpp backend (model + context) from scratch, then the context again.

My take on Claude's suggestions:

• bucketing context window sizes in hopes that calls can reuse the engine is good but wasteful, as it could lead to using context windows roughly two times too large in the case the input length lands right above the threshold
• skipping the context window size when comparing inputs is probably an okay thing to do because the backend already handles fitting the llama_context to the input

I'm not a fan of either. I'd rather measure the time it takes to create a context with a warm llama_model, as arena allocators in llama.cpp could make this very cheap.

• If the performance is acceptable, then I'd advise splitting LlamaBackend into two: the model and the context, just like llama.cpp does. With this split in place, we can have thread-safe reuse of the loaded models (this would be the "engine" in Inference Process terms), and have the context instanciated just in time, thrown away after use.
• If the performance is not acceptable, I'd still consider the split and caching the context in e.g. a thread-local, and make it "grow only". Thread-locals would be torn-down with the worker, so it would probably have to be made long-lived rather than ephemeral as it currently is.

Note that those suggestions are not Link Preview specific and could make future use of the llama.cpp backend more ergonomic.

Comment 4

[Greg Tatum [:gregtatum]]

Thanks for the detailed analysis. I'm making this block Bug 2038014 for on-device memory generation. We should be confident in our context management being efficient and correct.

Comment 5

[Valentin Pollet]

I did some quick measurements by adding markers around context creation (marker Llama:ContextAlloc) and model loading (marker Llama:ModelInit) in native code and profiling. I recorded three Link Previews.

-> The full profile

On my MacBook Pro (Apple M5 Pro) the breakdown is:

First run
Llama:ModelInit: 108ms
Llama:ContextAlloc: 2.2ms
Time to first 20 tokens (first Generate 20 tokens): 368ms
Total generation time: 758ms

Subsequent runs
Llama:ModelInit: 51, 56ms
Llama:ContextAlloc: 2.5, 2.7ms
Time to first 20 tokens (first Generate 20 tokens): 418, 456ms
Total generation time: 851, 779ms

I obtained similar profiles on my PC (Linux, AMD Ryzen 9950X).

While this is in no way statistically significant, we can safely assume that context initialization times are orders of magnitude less than model loading. We can also see that on Unix-based operating systems the using the mmap option from llama-cpp allows for warm-ish reloads after the first one (roughly half the time to load the model). Looking at code in llama.cpp, the benefits of mmap are likely cross-platform.

The profile gives us hints as to what the performance characteristics of llama.cpp backed engines in Firefox are. Perceived latency, tied to first 20 tokens, is an order of magnitude above model initialization, which is an order of magnitude above context initialization.

The motivation behind this issue is to keep the engine warm for Link Preview. The trade-off is, in my opinion, not worth it. We're talking about hogging 400-500MB of RAM between calls, to save 10-20% perceived latency, and closer to 10% overall generation time. Besides, introducing a time-to-live for Link Preview is really a product decision that has impacts on how AI use in Firefox is perceived by users. In light of this, I'd rather not go forward with this improvement, and https://bugzilla.mozilla.org/show_bug.cgi?id=1994488.

Looking at how cheap context initialization is, there's an opportunity to simplify a code quite a bit though, making LlamaBackend leaner and closer to thread-safety here. We could refactor LlamaBackend to not persist llama_context between calls and just have it transient, removing numContext from PipelineOptions. The only case where context reuse would be worth the engineering/maintenance effort would be KV cache reuse, which the LlamaBackend is a lot of engineering away from doing anyway.

Comment 6

[Greg Tatum [:gregtatum]]

Besides, introducing a time-to-live for Link Preview is really a product decision that has impacts on how AI use in Firefox is perceived by users.

I agree that we can just do the simplest and fastest thing here, and take it on as an engineering performance and maintenance decision.

:vpollet Do you mind closing out the relevant bugs as WONTFIX and then file the follow-up bugs here for your recommendation?