microsoft / onnxruntime

ONNX Runtime: cross-platform, high performance ML inferencing and training accelerator
https://onnxruntime.ai
MIT License
14.43k stars 2.9k forks source link

C API question - Custom allocator #6143

Open DvirDukhan opened 3 years ago

DvirDukhan commented 3 years ago

Hi Is there an option to create a custom allocator using the c API, to be shared across all sessions? From C_API.md:

So I wonder if it is possible to override the OrtAllocator struct Alloc and Free functions

typedef struct OrtAllocator {
  uint32_t version;  // Initialize to ORT_API_VERSION
  void*(ORT_API_CALL* Alloc)(struct OrtAllocator* this_, size_t size);
  void(ORT_API_CALL* Free)(struct OrtAllocator* this_, void* p);
  const struct OrtMemoryInfo*(ORT_API_CALL* Info)(const struct OrtAllocator* this_);
} OrtAllocator;

Thanks

pranavsharma commented 3 years ago

We don't have it today. We considered this while developing this API, but backed out as we wanted to settle on the correct OrtAllocator interface. I'll add it to our backlog.

snnn commented 3 years ago

I think the following line need be changed: https://github.com/microsoft/onnxruntime/blob/master/onnxruntime/core/providers/cpu/cpu_execution_provider.h#L46

To allow an allocator be passed in.

In general, I suggest our team read this: https://github.com/google/guice/wiki/GettingStarted

Instead of having code like:

class Foo {
  private Database database;  // We need a Database to do some work

  Foo() {
    // Ugh. How could I test this? What if I ever want to use a different
    // database in another application?
    this.database = new Database("/path/to/my/data");
  }
}

We would prefer

class Foo {
  private Database database;  // We need a Database to do some work

  // The database comes from somewhere else. Where? That's not my job, that's
  // the job of whoever constructs me: they can choose which database to use.
  Foo(Database database) {
    this.database = database;
  }
}

(I'm not saying we should introduce a dependency injection framework, I'm just suggesting we should follow such a pattern to make onnxruntime more extensible)

DvirDukhan commented 3 years ago

@pranavsharma any ETA on this? Does a pull request by me or my colleagues contributing to this feature is valid?

DvirDukhan commented 3 years ago

@pranavsharma My colleague @alonre24 submitted #6689. Can you please review it?

pranavsharma commented 3 years ago

Thanks for your contribution. We're reviewing and discussing the PR internally. Stay tuned.

gedoensmax commented 8 months ago

@harihrarans29 shouldn't this be closed due to your PR ?