Skip to content

Commit 0df9ee9

Browse files
larryliu0820facebook-github-bot
authored andcommitted
Clarify ownership of runner components (#10338)
Summary: The ownership of these components need some clarification. * `Module` should be shared by `TextDecoderRunner` and potentially `TextPrefiller` (or `ImagePrefiller` in multimodal runner). * `TextDecoderRunner` should be shared by the `TextPrefiller` and `TextTokenGenerator`. * `Tokenizer` should be owned by the `Runner` as well as `TextTokenGenerator`. Reviewed By: kirklandsign Differential Revision: D73399600
1 parent cfd1be3 commit 0df9ee9

File tree

5 files changed

+26
-4
lines changed

5 files changed

+26
-4
lines changed

examples/models/llama/runner/runner.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ Error Runner::load() {
9999
"Failed to load %s as a Tiktoken artifact, trying BPE tokenizer",
100100
tokenizer_path_.c_str());
101101
tokenizer_.reset();
102+
// @lint-ignore CLANGTIDY facebook-hte-Deprecated
102103
tokenizer_ = std::make_unique<::tokenizers::Llama2cTokenizer>();
103104
err = tokenizer_->load(tokenizer_path_);
104105
ET_CHECK_TK_OK_OR_RETURN_ERROR(

examples/models/llava/runner/llava_image_prefiller.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace example {
1818
class ET_EXPERIMENTAL LlavaImagePrefiller
1919
: public ::executorch::extension::llm::ImagePrefiller {
2020
public:
21-
LlavaImagePrefiller(::executorch::extension::Module* module)
21+
explicit LlavaImagePrefiller(::executorch::extension::Module* module)
2222
: ImagePrefiller(module){};
2323
/**
2424
* Prefill an LLM Module with the given image input.

extension/llm/runner/text_decoder_runner.h

+7-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
#include <executorch/extension/module/module.h>
1515
#include <executorch/extension/tensor/tensor.h>
1616
#include <executorch/runtime/platform/compiler.h>
17-
#include <functional>
1817

1918
namespace executorch {
2019
namespace extension {
@@ -94,7 +93,13 @@ class ET_EXPERIMENTAL TextDecoderRunner {
9493
}
9594

9695
protected:
97-
// TODO: use shared_ptr for module
96+
/**
97+
* Note: TextDecoderRunner does not own the Module instance. It is expected
98+
* that the outer class (likely Runner) manages the lifecycle of the Module.
99+
* This means that the responsibility for creating, maintaining, and
100+
* destroying the Module lies outside of TextDecoderRunner. Ensure that the
101+
* Module remains valid for the duration of TextDecoderRunner's usage.
102+
*/
98103
Module* module_;
99104
bool use_kv_cache_;
100105
bool should_stop_{false};

extension/llm/runner/text_prefiller.h

+9-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ class ET_EXPERIMENTAL TextPrefiller {
2424
bool use_kv_cache_,
2525
bool enable_parallel_prefill,
2626
int64_t max_seq_len = 128);
27+
28+
virtual ~TextPrefiller() = default;
2729
/**
2830
* Prefill an LLM Module with the given text input.
2931
* @param prompt_tokens The text prompt tokens to the LLM Module. Encoded by
@@ -32,7 +34,7 @@ class ET_EXPERIMENTAL TextPrefiller {
3234
* Module.
3335
* @return The next token of the LLM Module after prefill.
3436
*/
35-
::executorch::runtime::Result<uint64_t> prefill(
37+
virtual ::executorch::runtime::Result<uint64_t> prefill(
3638
std::vector<uint64_t>& prompt_tokens,
3739
int64_t& start_pos);
3840

@@ -48,6 +50,12 @@ class ET_EXPERIMENTAL TextPrefiller {
4850
int64_t& start_pos);
4951

5052
private:
53+
/**
54+
* Note: TextPrefiller does not own the TextDecoderRunner instance.
55+
* The responsibility of managing the lifecycle of TextDecoderRunner
56+
* lies with the outer class or entity (likely Runner) that creates
57+
* and passes the TextDecoderRunner instance to TextPrefiller.
58+
*/
5159
TextDecoderRunner* text_decoder_runner_;
5260
bool use_kv_cache_;
5361
bool enable_parallel_prefill_;

extension/llm/runner/text_token_generator.h

+8
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ class ET_EXPERIMENTAL TextTokenGenerator {
3232
use_kv_cache_(use_kv_cache),
3333
stats_(stats) {}
3434

35+
virtual ~TextTokenGenerator() = default;
36+
3537
/**
3638
* Token generation loop.
3739
* @param tokens prompt tokens as well as the first token generated by
@@ -136,6 +138,12 @@ class ET_EXPERIMENTAL TextTokenGenerator {
136138
}
137139

138140
private:
141+
/**
142+
* Note: TextTokenGenerator does not own the tokenizer_ and
143+
* text_decoder_runner_. The lifecycle of these objects should be managed
144+
* externally, likely in the Runner. This class assumes that the provided
145+
* pointers remain valid for the duration of its use.
146+
*/
139147
::tokenizers::Tokenizer* tokenizer_;
140148
TextDecoderRunner* text_decoder_runner_;
141149
std::unique_ptr<std::unordered_set<uint64_t>> eos_ids_;

0 commit comments

Comments
 (0)