-
Notifications
You must be signed in to change notification settings - Fork 712
[flat_tensor] Persist FreeableBuffers of external constants in method #8437
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
Changes from 2 commits
7362aad
9c61d73
de88fc3
bd639b1
04ab3b5
73cb971
644d057
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,17 @@ using OpFunction = void (*)(KernelRuntimeContext&, EValue**); | |
| /// argument list for a single instruction | ||
| using InstructionArgs = Span<EValue*>; | ||
|
|
||
| /// Data structure to hold key and data buffer for external data used | ||
| /// in a method. | ||
| struct NamedData { | ||
| const char* key; | ||
| FreeableBuffer* buffer; | ||
| }; | ||
lucylq marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // Check if key exists in external_constants. | ||
| // A helper function for parse_external_constants. | ||
lucylq marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| bool key_exists(const char* key, NamedData* external_constants, int num_keys); | ||
lucylq marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * An executable method of an executorch program. Maps to a python method like | ||
| * `forward()` on the original nn.Module. | ||
|
|
@@ -66,7 +77,9 @@ class Method final { | |
| delegates_(rhs.delegates_), | ||
| n_chains_(rhs.n_chains_), | ||
| chains_(rhs.chains_), | ||
| init_state_(rhs.init_state_) { | ||
| init_state_(rhs.init_state_), | ||
| external_constants_(rhs.external_constants_), | ||
| num_external_constants_(rhs.num_external_constants_) { | ||
| // Required: clear out fields that the dtor looks at, so that we don't free | ||
| // anything twice. | ||
| rhs.n_value_ = 0; | ||
|
|
@@ -84,6 +97,8 @@ class Method final { | |
| rhs.event_tracer_ = nullptr; | ||
| rhs.n_chains_ = 0; | ||
| rhs.chains_ = nullptr; | ||
| rhs.external_constants_ = nullptr; | ||
| rhs.num_external_constants_ = 0; | ||
lucylq marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -288,7 +303,9 @@ class Method final { | |
| delegates_(nullptr), | ||
| n_chains_(0), | ||
| chains_(nullptr), | ||
| init_state_(InitializationState::Uninitialized) {} | ||
| init_state_(InitializationState::Uninitialized), | ||
| external_constants_(nullptr), | ||
| num_external_constants_(0) {} | ||
|
|
||
| /// Static factory used by Program. | ||
| ET_NODISCARD static Result<Method> load( | ||
|
|
@@ -338,6 +355,23 @@ class Method final { | |
|
|
||
| InitializationState init_state_; | ||
|
|
||
| NamedData* external_constants_; | ||
| size_t num_external_constants_ = 0; | ||
|
||
|
|
||
| /** | ||
| * Counts the number of external constants for this method. | ||
| */ | ||
| ET_NODISCARD Result<size_t> get_num_external_constants(); | ||
|
|
||
| /** | ||
| * Parses the flatbuffer for constant tensors tagged as EXTERNAL. | ||
| * Retrieves the external constants using the named_data_map and places them | ||
| * into `external_constants_`. | ||
| * FreeableBuffers returned by the named_data_map are owned by the | ||
| * method and are freed on method destruction. | ||
| */ | ||
| ET_NODISCARD Error | ||
| parse_external_constants(const NamedDataMap* named_data_map); | ||
lucylq marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** | ||
| * Parses the elements of the values_ array. On error, n_value_ will be set to | ||
| * the number of successfully-initialized entries so that ~Method doesn't try | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| #include <executorch/runtime/core/evalue.h> | ||
| #include <executorch/runtime/core/exec_aten/exec_aten.h> | ||
| #include <executorch/runtime/executor/memory_manager.h> | ||
| #include <executorch/runtime/executor/method.h> | ||
| #include <executorch/runtime/executor/program.h> | ||
| #include <executorch/schema/program_generated.h> | ||
|
|
||
|
|
@@ -25,14 +26,21 @@ ET_NODISCARD Result<executorch::aten::Tensor> parseTensor( | |
| const Program* program, | ||
| MemoryManager* memory_manager, | ||
| const executorch_flatbuffer::Tensor* s_tensor, | ||
| const NamedDataMap* named_data_map = nullptr); | ||
| const NamedDataMap* named_data_map = nullptr, | ||
| Span<NamedData> external_constants = {}); | ||
|
|
||
| ET_NODISCARD Result<BoxedEvalueList<executorch::aten::Tensor>> parseTensorList( | ||
| const flatbuffers::Vector<int32_t>* tensor_indices, | ||
| EValue* values, | ||
| size_t values_len, | ||
| MemoryManager* memory_manager); | ||
|
|
||
| // Checks that the sizes, dim_order and scalar_type match between tensors | ||
| // stored in the PTE and externally. | ||
| ET_NODISCARD Error validateExternalTensor( | ||
| const executorch_flatbuffer::Tensor* s_tensor, | ||
| const TensorLayout& tensor_layout); | ||
lucylq marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // Deserializes a List of optional type. The code here is the same between all | ||
| // list of optionals: list of optional Tensor, list of optional float etc, so we | ||
| // just use a template to avoid boilerplate. | ||
|
|
@@ -105,7 +113,9 @@ parseListOptionalType( | |
| * @param[in] nbytes The amount of memory to get from the allocator. | ||
| * @param[in] allocator The source of memory for non-constant tensors. | ||
| * @param[in] named_data_map An optional map of {name, blob} used to resolve | ||
| * data that is external to the PTE, if any. | ||
| * data that is mutable and external to the PTE, if any. | ||
| * @param[in] external_constants An optional span of {name, buffer} used to | ||
|
||
| * resolve data that is constant and external to the PTE, if any. | ||
lucylq marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| * | ||
| * @returns On success, the data pointer to use for the tensor. On failure, a | ||
| * non-Ok Error. | ||
|
|
@@ -115,7 +125,8 @@ ET_NODISCARD Result<void*> getTensorDataPtr( | |
| const Program* program, | ||
| size_t nbytes, | ||
| HierarchicalAllocator* allocator, | ||
| const NamedDataMap* named_data_map = nullptr); | ||
| const NamedDataMap* named_data_map = nullptr, | ||
| Span<NamedData> external_constants = {}); | ||
|
|
||
| } // namespace deserialization | ||
| } // namespace runtime | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dbort I moved the check here, as this is now the first iteration over the flatbuffer values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see. In this case, I'd remove the comment here, since it doesn't actually describe the check's purpose: in this case, it's just ensuring that the value is non-null so it can be used here. "val_as_X()" isn't used in this function, so the comment will seem incongruous to readers.
Or you could reword it like "Note that as a side-effect of this check, we're guaranteed that all values are non-null, so later loops can skip that check."