Skip to content

Use System.arraycopy and Arrays.fill in CobolDataStorage.memcpy/memset - #877

Open
yutaro-sakamoto wants to merge 1 commit into
opensourcecobol:developfrom
yutaro-sakamoto:optimize-storage-memcpy-upstream
Open

Use System.arraycopy and Arrays.fill in CobolDataStorage.memcpy/memset#877
yutaro-sakamoto wants to merge 1 commit into
opensourcecobol:developfrom
yutaro-sakamoto:optimize-storage-memcpy-upstream

Conversation

@yutaro-sakamoto

Copy link
Copy Markdown
Contributor

#844 への対応。

概要

CobolDataStoragememcpy / memset のうち、setByte / getByte を使って1バイトずつコピー・フィルしていたオーバーロードを System.arraycopy / Arrays.fill に置き換えた。これらは JIT のイントリンシックであり、要素ごとの境界チェックとメソッド呼び出しのオーバーヘッドを避けられる。memcpy(byte[], int, int) はすでに System.arraycopy を使っており、今回の変更で他のオーバーロードもそれに揃う形になる。

対象:

  • memcpy(byte[] buf, int size)
  • memcpy(int offset, byte[] buf, int size)
  • memcpy(CobolDataStorage buf, int size)
  • memcpy(int offset, CobolDataStorage buf, int size)
  • memset(byte ch, int size)
  • memset(int offset, byte ch, int size)

this.index による相対アドレッシングはそのまま維持しているため、部分項目や OCCURS の扱いは変わらない。公開APIの変更もない。

付随する修正

System.arraycopy は長さが負の場合に例外を投げるが、置き換え前の for ループは何もしないだけだった。この差分によって、日本語項目への STRING ... WITH POINTER で異常終了する経路が1つ生じることが分かったため併せて修正した。

stringInitWITH POINTER の値を、日本語項目向けの2倍補正を行う に連結先のバイト数と比較している。そのため PIC N(5) に対して POINTER が 7〜10 の場合、補正後の stringOffset が連結先のバイト数(10)を超え、stringAppend で残り領域が負になっていた。従来はその memcpy が実質 no-op となりオーバーフロー状態になるだけだったが、System.arraycopy では ArrayIndexOutOfBoundsException で異常終了する。

stringAppend で残り領域が0以下の場合はコピーを行わないようにし、従来と同じ結果(何も連結せずオーバーフロー条件を設定、文字位置は連結先の末尾)になるようにした。

また、memcpy(byte[] buf, int offset, int size)@param offset の説明が実装(コピー元 buf 内のオフセット)と食い違っていたため修正した。

テスト

  • CobolDataStorageTest を新規追加。変更した各オーバーロードについて、相対位置(index)が維持されること、書き込み範囲外が変化しないことを確認する。
  • CobolStringTest を新規追加。上記 STRING の回帰テストを含む(stringAppend のガードが無い状態では失敗することを確認済み)。
  • CI は全ジョブ成功。

Addresses #844.

Summary

Several memcpy / memset overloads in CobolDataStorage copied or filled the backing array one byte at a time through setByte / getByte. They now use System.arraycopy and Arrays.fill, which are JIT intrinsics and avoid the per-element bounds checks and call overhead. memcpy(byte[], int, int) already used System.arraycopy, so the remaining overloads are now consistent with it.

Changed overloads:

  • memcpy(byte[] buf, int size)
  • memcpy(int offset, byte[] buf, int size)
  • memcpy(CobolDataStorage buf, int size)
  • memcpy(int offset, CobolDataStorage buf, int size)
  • memset(byte ch, int size)
  • memset(int offset, byte ch, int size)

Relative addressing through this.index is preserved, so sub-fields and OCCURS items behave exactly as before. There is no public API change.

Accompanying fix

System.arraycopy throws on a negative length, whereas the previous for loop simply did nothing. That difference exposed one path where STRING ... WITH POINTER into a national item would abort.

stringInit compares the WITH POINTER value against the destination size in bytes before doubling the offset for national items. For a PIC N(5) item and a pointer of 7 through 10, the doubled stringOffset ends up past the item's 10 bytes, so stringAppend computed a negative remaining size. Previously that memcpy was effectively a no-op and the statement just raised the overflow condition; with System.arraycopy it aborted with ArrayIndexOutOfBoundsException.

stringAppend now skips the copy when nothing fits, which reproduces the previous outcome: nothing is appended, the overflow condition is set, and the pointer ends at the end of the destination.

The @param offset of memcpy(byte[] buf, int offset, int size) also described the destination, while the implementation uses it as an offset into the source array; the Javadoc is corrected.

Tests

  • New CobolDataStorageTest covering each changed overload: relative addressing via index is preserved and bytes outside the written range are untouched.
  • New CobolStringTest, including a regression test for the STRING case above (verified to fail without the stringAppend guard).
  • CI is green.

このPRの内容は yutaro-sakamoto#33 と同一です(ベースブランチのみ異なります)。

The contents of this pull request are identical to yutaro-sakamoto#33; only the base branch differs.

The remaining memcpy/memset overloads copied or filled the backing array
one byte at a time through setByte/getByte. Replace those loops with
System.arraycopy and Arrays.fill, which are JIT intrinsics and avoid the
per-element bounds checks and call overhead. This matches
memcpy(byte[], int, int), which already used System.arraycopy.

The relative addressing based on this.index is preserved, so sub-fields
and OCCURS items behave as before.

System.arraycopy throws on a negative length while the per-byte loop
silently did nothing, and that difference exposed one path that aborted.
stringInit compares WITH POINTER against the destination size in bytes
before doubling the offset for national items, so for a PIC N(n) item a
pointer between n/2 and n leaves stringOffset past the end of the item
and stringAppend computed a negative remaining size. Skip the copy when
nothing fits and keep setting the overflow condition, which is what the
previous implementation effectively did.

Also fix the @PARAM offset of memcpy(byte[], int, int), which describes
an offset into the source array, not the destination.

Add unit tests for the changed overloads and for the STRING case above.
@yutaro-sakamoto
yutaro-sakamoto marked this pull request as ready for review August 5, 2026 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant