[
https://issues.apache.org/jira/browse/MATH-1306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15071775#comment-15071775
]
Gilles commented on MATH-1306:
------------------------------
bq. Memory allocation takes time
There are usage patterns where allocation is more efficient than reusing
objects. I don't know whether it is the case here or in the use-case you have
in mind for using this feature.
bq. Because this is one of the sings one needs to do when the second signature
of nextBytes() isn't awailable.
Since the new feature is about reusing the same array to fill different regions
of it at different times, why not also keep the small array for reuse?
bq. After this temporary byte array is used and forgotten it needs to be freed
by the garbage collector.
Indeed, and I strongly suspect that your are benchmarking the GC and not the
competing implementations ("fill at offset" vs "fill and copy").
bq. Isn't the performance and memory impact just obvious?
If you take the memory allocation out of the loop, no.
If most usages would fill the entire array, then the additional function call
might slow down things. This is just as "obvious" and maybe as wrong as any
conclusion from micro-benchmarks in Java. ;)
bq. If the second signature of the nextBytes() method is available the user
code could also be simpler.
Could you please provide a use-case of such a simplification? I only see that
the signature is more complex; the purpose of Commons Math is not to cover all
hypothetical cases.
In case one needs to fill a large array (that is anyways in memory since it is
going to reused), I don't see why not do a single call to "nextBytes(array)" to
fill it all and then fetch regions of it.
And if relatively smaller regions are used one at a time, why allocate a big
array rather than one corresponding to the largest region?
But I may be missing something; hence my request for clarification...
> Add public void nextBytes(byte[] bytes, int position, int length) method into
> the RandomGenerator interface
> -----------------------------------------------------------------------------------------------------------
>
> Key: MATH-1306
> URL: https://issues.apache.org/jira/browse/MATH-1306
> Project: Commons Math
> Issue Type: Improvement
> Affects Versions: 3.5
> Reporter: Rostislav Krasny
> Attachments: MersenneTwister1305.java, MersenneTwister1306.java
>
>
> I propose to add {{public void nextBytes(byte[] bytes, int position, int
> length)}} method into the {{RandomGenerator}} interface.
> Rationality: to improve performance and memory usage in cases when one needs
> to fill only a specified region of a byte array. Today to do that you need to
> use a temporary byte array and copy it by yourself into the specified region
> of the destination byte array.
> I propose the following code, based on the code of {{BitsStreamGenerator}}
> commited in MATH-1305
> {code:java}
> @Override
> public void nextBytes(byte[] bytes) {
> nextBytesFill(bytes, 0, bytes.length);
> }
> // TODO add this method into RandomGenerator interface
> //@Override
> public void nextBytes(byte[] bytes, int position, int length) {
> if (position < 0 || position > bytes.length - 1) {
> throw new
> OutOfRangeException(LocalizedFormats.OUT_OF_RANGE_SIMPLE, position, 0,
> bytes.length - 1);
> }
> if (length < 0 || length > bytes.length - position) {
> throw new
> OutOfRangeException(LocalizedFormats.OUT_OF_RANGE_SIMPLE, length, 0,
> bytes.length - position);
> }
> nextBytesFill(bytes, position, length);
> }
> private void nextBytesFill(byte[] bytes, int position, int length) {
> int index = position;
> // Position plus multiple 4 part of length (i.e. length with
> two least significant bits unset).
> final int indexLoopLimit = position + (length & 0x7ffffffc);
> // Start filling in the byte array, 4 bytes at a time.
> while (index < indexLoopLimit) {
> final int random = next(32);
> bytes[index++] = (byte) random;
> bytes[index++] = (byte) (random >>> 8);
> bytes[index++] = (byte) (random >>> 16);
> bytes[index++] = (byte) (random >>> 24);
> }
> final int indexLimit = position + length;
>
> // Fill in the remaining bytes.
> if (index < indexLimit) {
> int random = next(32);
> while (true) {
> bytes[index++] = (byte) random;
> if (index < indexLimit) {
> random >>>= 8;
> } else {
> break;
> }
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
