C_APIのバッファのクローン(memcopy)を減らす

[higumachan]

内容

現状はc-apiにて https://github.com/VOICEVOX/voicevox_core/blob/a787f6d0f4bc111568114801ac38ca826320c339/crates/voicevox_core_c_api/src/lib.rs#L190-L253)

https://github.com/VOICEVOX/voicevox_core/blob/a787f6d0f4bc111568114801ac38ca826320c339/crates/voicevox_core_c_api/src/helpers.rs#L108-L118

https://github.com/VOICEVOX/voicevox_core/blob/a787f6d0f4bc111568114801ac38ca826320c339/crates/voicevox_core_c_api/src/helpers.rs#L128-L139

のように，Rustから取得されるバッファをlibc::mallocで確保し直したバッファにコピーしています．

Rust側で確保されているバッファのポインタをそのまま返すようにすると，コピーの回数が減るかと思います．

Pros 良くなる点

• 瞬間的なバッファの使用量が半分になる
• バッファのコピーが一つ減るので高速になる(可能性が高い)
• unsafeな関数の呼び出し回数が減る

Cons 悪くなる点

• 読むときに必要なRustの知識が増える(主観的な話ですが)

実現方法

RustのBox::leakを利用すると以下のようにかけます．

#[no_mangle]
pub unsafe extern "C" fn voicevox_predict_duration(
    length: usize,
    phoneme_vector: *mut i64,
    speaker_id: u32,
    output_predict_duration_data_length: *mut usize,
    output_predict_duration_data: *mut *mut f32,
) -> VoicevoxResultCode {
    into_result_code_with_error((|| {
        let output_vec = lock_internal().predict_duration(
            std::slice::from_raw_parts_mut(phoneme_vector, length),
            speaker_id,
        )?;
        let boxed_slice = output_vec.into_boxed_slice();
        output_predict_duration_data_length.write(boxed_slice.len());
        output_predict_duration_data.write(Box::leak(boxed_slice).as_mut_ptr());

        Ok(())
    })())
}

#[no_mangle]
pub unsafe extern "C" fn voicevox_predict_duration_data_free(predict_duration_data: *mut f32) {
    Box::from_raw(predict_duration_data);
}

指摘頂いた点を修正したので追記:

#[no_mangle]
pub unsafe extern "C" fn voicevox_predict_duration_data_free(predict_duration_data: *mut f32, predict_duration_data_length: usize) {
    drop(Box::new(slice::from_raw_parts(predict_duration_data, predict_duration_data_length)));
}

OSの種類/ディストリ/バージョン

その他

[qryxip]

確かにRust側でもfreeはできそうですね。

ただそのBox::from_rawですが、Box<f32>になって先頭の一要素しか解放されない気がします。lengthも要求してVec::from_raw_partsを使う形になるのではないかと思います。

[higumachan]

ただそのBox::from_rawですが、Boxになって先頭の一要素しか解放されない気がします。lengthも要求してVec::from_raw_partsを使う形になるのではないかと思います。

確かにそのとおりでした． こんな感じですかね．

#[no_mangle]
pub unsafe extern "C" fn voicevox_predict_duration_data_free(predict_duration_data: *mut f32, predict_duration_data_length: usize) {
    drop(Box::new(slice::from_raw_parts(predict_duration_data, predict_duration_data_length)));
}

ただ，APIが変わってしまうため議論しなければ行けないポイントは増えますね．

API合わせるなら，Rust内部でPointerから引けるHashMap的なStateを用意する感じかなと思います．

[Hiroshiba]

issue作成ありがとうございます！！たしかにメモリコピーを一度減らせそうだと感じました！

パラメータにlengthが追加されると、ユーザーにとってはAPIが変わるだけでなくその値を保持する必要があって少し大変になりそうだなと感じました。 そこはライブラリ側で持ってあげる形、つまり仰るとおりMapを作ることで結構きれいに解決できるのかなと感じました！！

[qryxip]

Rustのアロケータ自身はサイズを覚えていないのかと思ったのですが、どうやら覚えていないみたいですね。 Mapで持つのがusizeかstd::alloc::Layoutかの違いだけになりそうですが、後者の方が意図が明確になるかも?

[qryxip]

いや内部はglibcのmallocとかを呼んでいたと思うので、中途半端に使ったら危ないかもしれません。usizeの方がいいかも。

追記: いやfrom_raw(_parts)でも変わらないかも

[higumachan]

すこし方針を変えて，Boxを経由せずにVec::leakを利用して実装するほうが安全そうだったのでこちらで実装を行いました．