stdlib icon indicating copy to clipboard operation
stdlib copied to clipboard

feat: add functional equivalent of `Array.prototype.copyWithin` and `TypedArray.prototype.copyWithin`

Open ShraddheyaS opened this issue 4 years ago • 3 comments

Resolves #203.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

  • [X] Read, understood, and followed the contributing guidelines, including the relevant style guides.
  • [X] Read and understand the Code of Conduct.
  • [X] Read and understood the licensing terms.
  • [X] Searched for existing issues and pull requests before submitting this pull request.
  • [X] Filed an issue (or an issue already existed) prior to submitting this pull request.
  • [X] Rebased onto latest develop.
  • [X] Submitted against develop branch.

Description

What is the purpose of this pull request?

This pull request:

Related Issues

Does this pull request have any related issues?

This pull request:

  • resolves #203

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.


@stdlib-js/reviewers

ShraddheyaS avatar Sep 28 '20 19:09 ShraddheyaS

@ShraddheyaS Thanks for the PR! copyWithin is actually more interesting than I anticipated. Notably, the algo you used (akin to the MDN polyfill implementation) is good for the generic case where you need to account for sparse array-like objects. However, we can do more interesting things with Typed Arrays. Notably, for, e.g., Uint8Array inputs, we could copy in 4 byte increments (i.e., interpret the underlying array buffer as a Uint32Array) plus any remaining bytes. In C, we could even interpret as double, I believe, but we cannot do that in JavaScript due to canonicalized NaNs.

Also, did you include in your test cases whether the polyfill properly accounts for overlapping buffers? Not clear to me, from the algo, whether this is true. I would expect in those scenarios that we’d need to copy-and-paste, but maybe I am missing something from the algo.

kgryte avatar Sep 28 '20 20:09 kgryte

@kgryte Yes, that does sound more performant instead of copying individual byte-sized elements for the Uint8Array inputs.

By overlapping buffers, do you mean the case where the target is after start in which the portion to be copied is going to be overwritten at the end of the operation? If yes, then there is a test case for that scenario. The algorithm handles that by copying elements in the reverse direction (steps 16-17 in the MDN polyfill).

count = min( final-from, len-to );
direction = 1;
if ( from < to && to < ( from+count ) ) {
	direction = -1;
	from += count - 1;
	to += count - 1;
}

Processing Uint8Array inputs differently would mean handling the case of overlapping buffers in a slightly different way around the target bytes in above.

ShraddheyaS avatar Sep 29 '20 10:09 ShraddheyaS

@ShraddheyaS Re: overlapping buffers. Thanks for the clarification.

Re: algo. Yeah, I am not sure, atm, whether the potential perf gains would pan out, but seems like something we may want to investigate.

Regardless, I think we may want to treat typed arrays differently than generic arrays and array-like objects. Notably, the from in collection is not generally applicable to typed arrays, as they are contiguous blocks of memory. We don't need to perform a look-up for the property before copying. This is mainly applicable for sparse arrays.

Presumably, we could also treat generic arrays as being dense. Sparse generic arrays are not particularly common, imo, and have perf cliffs. We may only want to apply sparse treatment to array-like objects. This would, however, cause us to differ from the spec, which I am fine with, so long as we have reasonable justification.

kgryte avatar Oct 01 '20 03:10 kgryte