perf: 非同期処理を改良

[sabonerune]

内容

CharacterInfoの作成をより並行に実行するように変更します。

また、ソングのみのキャラクター対応も行います。

その他

ソングのみのキャラクター対応はdiffArrayを使用して一方しかなくても順序が維持されるようにしました。
しかし、一方のみのキャラクターが交互に続いた場合の対応は不可能です。

/speakersと/singersや/speaker_infoと/singer_infoを同時にリクエストするようにしたり、リクエスト処理の間にStyleInfoの作成を進めたりするようにしました。(base64のデコードがやや重い処理のようです)

ただ可読性が下がってしまい自分にはこれ以上なんとかできませんでした。

[Hiroshiba]

ほぼLGTMです！！
元のコード（僕が書きました。。）よりずっとわかりやすいと感じました！！

[Hiroshiba]

diffArray使うのも良さそうですが、diffライブラリに依存しちゃうのはなるべく避けておきたいかもです。
（爆速なら話は別ですが、まあデータ数も少なそうですし）

Setを使うのはどうでしょう。こんな感じ？

const speakerUuids = new Set(speakers.map(speaker => speaker.speakerUuid));
const singerUuids = new Set(singers.map(singer => singer.speakerUuid));
const allUuids = new Set([...speakerUuids, ...singerUuids]);
const characterInfosPromise = Array.from(allUuids).map(speakerUuid => {
  const isSpeaker = speakerUuids.has(speakerUuid);
  const isSinger = singerUuids.has(speakerUuid);
  const speaker = isSpeaker ? speakers.find(speaker => speaker.speakerUuid === speakerUuid) : undefined;
  const singer = isSinger ? singers.find(singer => singer.speakerUuid === speakerUuid) : undefined;
  return getCharacterInfo(speaker, singer);
});

getCharacterInfoがundefined渡せる前提なら、もっと簡略化して、こう･･･？

const allUuids = new Set([...speakers, ...singers].map(item => item.speakerUuid));
const characterInfosPromise = Array.from(allUuids).map(speakerUuid => {
  const speaker = speakers.find(speaker => speaker.speakerUuid === speakerUuid);
  const singer = singers.find(singer => singer.speakerUuid === speakerUuid);
  return getCharacterInfo(speaker, singer);
});

あ、これcharacterInfosPromiseじゃなくてcharacterInfoPromisesかもですね

[sabonerune]

Setを使うとトークのみ、ソングのみのキャラクターの順序の維持ができなくなりますね…

const speakers = ["a", "b", "c", "e"];
const singers = [ "a", "c", "d", "e"];
const characters = Array.from(new Set([...speakers, ...singers]));
console.log(characters); // [ 'a', 'b', 'c', 'e', 'd' ] "c"と"e"の間にあるべき"d"が一番後ろになる

ソングのみのキャラクターが一番後ろになっても構わないのならSetを使うのがいいと思います。

順序を維持してマージをする処理を自分で書くのは自分では無理だと判断したのとdiffが別のところで使われていることを思い出したので使用しました。

[Hiroshiba]

あーーーーーーーーなるほどです！！！　気づいてくださってありがとうございます 🙇 🙇 🙇
将来忘れそうなので、コメントで「順番を維持しつつキャラクター情報を取得」とか書くのお願いできると。。 🙇

今気づきましたが、diffだとsingersが例えばe, aだったときバグりそうですね･･･。

ちょっと効率の悪いコードな気がしますが、speakersに未知singers足す形が手っ取り早い気がしました。
こんな感じ？

// 順番を維持しつつキャラクター情報を取得
const speakerUuids = speakers.map((speaker) => speaker.speakerUuid)
const singerUuids = singers.map((singer) => singer.singerUuid)
// 配列を足すtsコードこれで合ってるのか自信ない。。
[...speakerUuids, ...singerUuids.filter(uuid=>!speakerUuids.has(uuid))].map(uuid => {
  // promiseを返すコード
})

[sabonerune]

このコードだと結局Setを使ったコードと同じになってしまう気が…

今気づきましたが、diffだとsingersが例えばe, aだったときバグりそうですね･･･。

const speakers = ["a", "b", "d"]
const singers = ["c", "a", "b", "d"]

なら ["c", "a", "b", "d"]になるはずです。

ただ、両方に存在するが位置関係が一致しない場合はバグになりますね…

const speakers = ["a", "b", "c", "e"];
const singers = [ "e", "a", "c", "d"];
// 多分 [ "e", "a", "b", "c", "e", "d"] になる

現在のVOICEVOXエンジンの場合は問題ありませんが今後のことやサードパーティ製エンジンのことも考えるとSetしかないですね。

[sevenc-nanashi]

CoreでSpeakerに順番の情報が入ったので、やるならこれをEngineでも吐くようにしてそれをベースに並び替えとかですかね？

[Hiroshiba]

@sevenc-nanashi
たしかに、だいぶ先の話になりそうですが順番入れても良いかもですね！！
というか入れとかないとこうなる。

@sabonerune

このコードだと結局Setを使ったコードと同じになってしまう気が…

あーーーーなるほどです！！！
歌手のみが最後になるの、なるほどです。問題を何かと勘違いしていました。

ちょっとどうしようもなさそうですね…！！
一旦今はそういう問題が起こりうる、という認識で進めるしかなさそう。
ちょっと一言fixmeコメントと、この議論へのリンクをコメントお願いできると🙇

そもそも全キャラ情報を一つの配列で持ってるのが問題の根本な気がしました。
話者と歌手別々に配列持っといて、複合したのが必要なら毎回合成すれば良さそう。（現状たぶん合成がいる箇所は無い）
…というissueをPRマージ時に立てると良さそう！忘れそうだけど！

[Hiroshiba]

あ、ちなみにsetはたぶん順番が変わりうるので微妙かもです！
順番変わらないというドキュメントがあればその前提で使ってもよさそう。（たしかMapにはそういうドキュメントが書かれてある）

[sevenc-nanashi]

https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/Set

挿入順は、各要素が add メソッドによって正常に Set に挿入された順番に対応します。

順番は保存されそう？（知らなかった）

[sabonerune]

Setで順番は変わらないみたいなのでSetを使いました。

[Hiroshiba]

LGTM！！

手元で測った感じ、もしかしたら実行時間が10%未満ほど伸びてるかもでした。
まあエンジン側に同時リクエストが走るので伸びていても不思議ではないかもですが、そもそもアーキテクチャーをなんとかするべきなのと、コードは見やすくなっているのとで、マージするのがいいのかなと思いました！

ちょっとこちらで一部変更させていただいてからマージしたいと思います！