IntPtr[] inputValuesArray = GetOrtValuesHandles(inputValues, true);

Recommendation:

instead of true, false you can do
const bool descriptive_name_true = true;

and pass it as an argument.

This would improve readability a lot.

In reply to: 1709295692

In reply to: 1709295692

In reply to: 1709295692

Refers to: csharp/src/Microsoft.ML.OnnxRuntime/Training/TrainingSession.shared.cs:365 in df21a2e. [](commit_id = df21a2e, deletion_comment = False)

        IntPtr[] inputValuesArray = GetOrtValuesHandles(inputValues, true);

Also, I am suggesting to introduce a OrtValue based API some time in the near future.

In reply to: 1709295692

Refers to: csharp/src/Microsoft.ML.OnnxRuntime/Training/TrainingSession.shared.cs:365 in df21a2e. [](commit_id = df21a2e, deletion_comment = False)

            Marshal.Copy(propertyValue, value, 0, 1);

Suggestion: float type is blittable, so you can simply copy it from native memory in the unsafe block and no array is needed, bc it introduces garbage. Marshal.Copy is slow.

Same is for Int

Something like this:
https://github.com/microsoft/onnxruntime/blob/main/csharp/src/Microsoft.ML.OnnxRuntime/NativeOnnxValueHelper.shared.cs#L219

Or one can do unsafe block:

float val;
unsafe
{
    val = \*(float\*)propertyValue.ToPointer();
}


---
In reply to: [1709298903](https://github.com/microsoft/onnxruntime/pull/17364#issuecomment-1709298903) [](http://example.com/codeflow?ancestors=1709298903)

---
In reply to: [1709298903](https://github.com/microsoft/onnxruntime/pull/17364#issuecomment-1709298903) [](http://example.com/codeflow?ancestors=1709298903)

---
In reply to: [1709298903](https://github.com/microsoft/onnxruntime/pull/17364#issuecomment-1709298903) [](http://example.com/codeflow?ancestors=1709298903)

---
Refers to: csharp/src/Microsoft.ML.OnnxRuntime/Training/CheckpointState.shared.cs:183 in df21a2e. [](commit_id = df21a2e20e52436297950511058c3626203d2e92, deletion_comment = False)

        }

See inference code for a function that converts and copies the string to native memory at the same time, and avoids intermediate array allocation.

https://github.com/microsoft/onnxruntime/blob/main/csharp/src/Microsoft.ML.OnnxRuntime/NativeOnnxValueHelper.shared.cs#L47

In reply to: 1709299587

In reply to: 1709299587

In reply to: 1709299587

Refers to: csharp/src/Microsoft.ML.OnnxRuntime/Training/CheckpointState.shared.cs:152 in df21a2e. [](commit_id = df21a2e, deletion_comment = False)

        using (var memHandle = memory.Pin())

nit: Pin() is very expensive.
fixed() block is cheap.

In reply to: 1724556619

Refers to: csharp/src/Microsoft.ML.OnnxRuntime/Training/CheckpointState.shared.cs:49 in 58ff40b. [](commit_id = 58ff40b, deletion_comment = False)

Make it to pass everything in a form of a string, and then convert to the type.

Refers to: orttraining/orttraining/training_api/include/onnxruntime_training_c_api.h:628 in 58ff40b. [](commit_id = 58ff40b, deletion_comment = False)

Thank you for the review @yuslepukhin @AdamLouly @pengwa :)