[data loader] move logic for FD data loader out of file_data_loader (#6682)

move fd loader

Co-authored-by: lind <[email protected]>

Author: davidlin54

examples/portable/executor_runner/executor_runner.cpp

@@ -26,6 +26,7 @@
 #include <unistd.h>
 
 #include <executorch/extension/data_loader/file_data_loader.h>
+#include <executorch/extension/data_loader/file_descriptor_data_loader.h>
 #include <executorch/extension/evalue_util/print_evalue.h>
 #include <executorch/extension/runner_util/inputs.h>
 #include <executorch/runtime/executor/method.h>
@@ -45,6 +46,7 @@ DEFINE_bool(
     "True if the model_path passed is a file descriptor with the prefix \"fd:///\".");
 
 using executorch::extension::FileDataLoader;
+using executorch::extension::FileDescriptorDataLoader;
 using executorch::runtime::Error;
 using executorch::runtime::EValue;
 using executorch::runtime::HierarchicalAllocator;
@@ -56,6 +58,33 @@ using executorch::runtime::Program;
 using executorch::runtime::Result;
 using executorch::runtime::Span;
 
+static Result<Program> getProgram(
+    const bool is_fd_uri,
+    const char* model_path) {
+  // Create a loader to get the data of the program file. This demonstrates both
+  // FileDataLoader and FileDescriptorDataLoader. There are other DataLoaders
+  // that use mmap() or point to data that's already in memory, and users can
+  // create their own DataLoaders to load from arbitrary sources.
+  if (!is_fd_uri) {
+    Result<FileDataLoader> loader = FileDataLoader::from(model_path);
+
+    ET_CHECK_MSG(
+        loader.ok(),
+        "FileDataLoader::from() failed: 0x%" PRIx32,
+        (uint32_t)loader.error());
+    return Program::load(&loader.get());
+  } else {
+    Result<FileDescriptorDataLoader> loader =
+        FileDescriptorDataLoader::fromFileDescriptorUri(model_path);
+
+    ET_CHECK_MSG(
+        loader.ok(),
+        "FileDescriptorDataLoader::fromFileDescriptorUri() failed: 0x%" PRIx32,
+        (uint32_t)loader.error());
+    return Program::load(&loader.get());
+  }
+}
+
 int main(int argc, char** argv) {
   executorch::runtime::runtime_init();
 
@@ -75,18 +104,9 @@ int main(int argc, char** argv) {
   const char* model_path = FLAGS_model_path.c_str();
   const bool is_fd_uri = FLAGS_is_fd_uri;
 
-  Result<FileDataLoader> loader = is_fd_uri
-      ? FileDataLoader::fromFileDescriptorUri(model_path)
-      : FileDataLoader::from(model_path);
-
-  ET_CHECK_MSG(
-      loader.ok(),
-      "FileDataLoader::from() failed: 0x%" PRIx32,
-      (uint32_t)loader.error());
-
   // Parse the program file. This is immutable, and can also be reused between
   // multiple execution invocations across multiple threads.
-  Result<Program> program = Program::load(&loader.get());
+  Result<Program> program = getProgram(is_fd_uri, model_path);
   if (!program.ok()) {
     ET_LOG(Error, "Failed to parse model file %s", model_path);
     return 1;

examples/portable/executor_runner/targets.bzl

@@ -15,6 +15,7 @@ def define_common_targets():
         deps = [
             "//executorch/runtime/executor:program",
             "//executorch/extension/data_loader:file_data_loader",
+            "//executorch/extension/data_loader:file_descriptor_data_loader",
             "//executorch/extension/evalue_util:print_evalue",
             "//executorch/extension/runner_util:inputs",
         ],

extension/data_loader/file_data_loader.cpp

@@ -43,8 +43,6 @@ namespace extension {
 
 namespace {
 
-static constexpr char kFdFilesystemPrefix[] = "fd:///";
-
 /**
  * Returns true if the value is an integer power of 2.
  */
@@ -76,36 +74,25 @@ FileDataLoader::~FileDataLoader() {
   ::close(fd_);
 }
 
-static Result<int> getFDFromUri(const char* file_descriptor_uri) {
-  // check if the uri starts with the prefix "fd://"
+Result<FileDataLoader> FileDataLoader::from(
+    const char* file_name,
+    size_t alignment) {
   ET_CHECK_OR_RETURN_ERROR(
-      strncmp(
-          file_descriptor_uri,
-          kFdFilesystemPrefix,
-          strlen(kFdFilesystemPrefix)) == 0,
+      is_power_of_2(alignment),
       InvalidArgument,
-      "File descriptor uri (%s) does not start with %s",
-      file_descriptor_uri,
-      kFdFilesystemPrefix);
-
-  // strip "fd:///" from the uri
-  int fd_len = strlen(file_descriptor_uri) - strlen(kFdFilesystemPrefix);
-  char fd_without_prefix[fd_len + 1];
-  memcpy(
-      fd_without_prefix,
-      &file_descriptor_uri[strlen(kFdFilesystemPrefix)],
-      fd_len);
-  fd_without_prefix[fd_len] = '\0';
+      "Alignment %zu is not a power of 2",
+      alignment);
 
-  // check if remaining fd string is a valid integer
-  int fd = ::atoi(fd_without_prefix);
-  return fd;
-}
+  // Use open() instead of fopen() to avoid the layer of buffering that
+  // fopen() does. We will be reading large portions of the file in one shot,
+  // so buffering does not help.
+  int fd = ::open(file_name, O_RDONLY);
+  if (fd < 0) {
+    ET_LOG(
+        Error, "Failed to open %s: %s (%d)", file_name, strerror(errno), errno);
+    return Error::AccessFailed;
+  }
 
-Result<FileDataLoader> FileDataLoader::fromFileDescriptor(
-    const char* file_name,
-    const int fd,
-    size_t alignment) {
   // Cache the file size.
   struct stat st;
   int err = ::fstat(fd, &st);
@@ -132,47 +119,6 @@ Result<FileDataLoader> FileDataLoader::fromFileDescriptor(
   return FileDataLoader(fd, file_size, alignment, file_name_copy);
 }
 
-Result<FileDataLoader> FileDataLoader::fromFileDescriptorUri(
-    const char* file_descriptor_uri,
-    size_t alignment) {
-  ET_CHECK_OR_RETURN_ERROR(
-      is_power_of_2(alignment),
-      InvalidArgument,
-      "Alignment %zu is not a power of 2",
-      alignment);
-
-  auto parsed_fd = getFDFromUri(file_descriptor_uri);
-  if (!parsed_fd.ok()) {
-    return parsed_fd.error();
-  }
-
-  int fd = parsed_fd.get();
-
-  return fromFileDescriptor(file_descriptor_uri, fd, alignment);
-}
-
-Result<FileDataLoader> FileDataLoader::from(
-    const char* file_name,
-    size_t alignment) {
-  ET_CHECK_OR_RETURN_ERROR(
-      is_power_of_2(alignment),
-      InvalidArgument,
-      "Alignment %zu is not a power of 2",
-      alignment);
-
-  // Use open() instead of fopen() to avoid the layer of buffering that
-  // fopen() does. We will be reading large portions of the file in one shot,
-  // so buffering does not help.
-  int fd = ::open(file_name, O_RDONLY);
-  if (fd < 0) {
-    ET_LOG(
-        Error, "Failed to open %s: %s (%d)", file_name, strerror(errno), errno);
-    return Error::AccessFailed;
-  }
-
-  return fromFileDescriptor(file_name, fd, alignment);
-}
-
 namespace {
 /**
  * FreeableBuffer::FreeFn-compatible callback.

extension/data_loader/file_data_loader.h

@@ -26,27 +26,6 @@ namespace extension {
  */
 class FileDataLoader final : public executorch::runtime::DataLoader {
  public:
-  /**
-   * Creates a new FileDataLoader that wraps the named file descriptor, and the
-   * ownership of the file descriptor is passed. This helper is used when ET is
-   * running in a process that does not have access to the filesystem, and the
-   * caller is able to open the file and pass the file descriptor.
-   *
-   * @param[in] file_descriptor_uri File descriptor with the prefix "fd:///",
-   *     followed by the file descriptor number.
-   * @param[in] alignment Alignment in bytes of pointers returned by this
-   *     instance. Must be a power of two.
-   *
-   * @returns A new FileDataLoader on success.
-   * @retval Error::InvalidArgument `alignment` is not a power of two.
-   * @retval Error::AccessFailed `file_name` could not be opened, or its size
-   *     could not be found.
-   * @retval Error::MemoryAllocationFailed Internal memory allocation failure.
-   */
-  static executorch::runtime::Result<FileDataLoader> fromFileDescriptorUri(
-      const char* file_descriptor_uri,
-      size_t alignment = alignof(std::max_align_t));
-
   /**
    * Creates a new FileDataLoader that wraps the named file.
    *
@@ -100,11 +79,6 @@ class FileDataLoader final : public executorch::runtime::DataLoader {
       void* buffer) const override;
 
  private:
-  static executorch::runtime::Result<FileDataLoader> fromFileDescriptor(
-      const char* file_name,
-      const int fd,
-      size_t alignment = alignof(std::max_align_t));
-
   FileDataLoader(
       int fd,
       size_t file_size,