On Fri, 16 Feb 2018 08:33:46 -0800, Chas Honton wrote:
Read Bloch. Don’t optimize until you have proven that this bit of
code is causing a significant performance hit.
arrayCopy can and is inlined by jit
https://www.artima.com/intv/bloch13.html
Bottom-line: Don't use clone except to copy arrays.
So I'd rephrase my comment on the commit: let "clone()" in place
but with the comment that it's the only acceptable use of it.
Regards,
Gilles
Chas
On Feb 16, 2018, at 5:53 AM, Stian Soiland-Reyes <st...@apache.org>
wrote:
String is still a type of Object (requiring GC handling of reference
counters), as you can do Object[] o = stringArray.clone(); without
casting. (other way not so)
https://www.javaspecialists.eu/archive/Issue124.html says there is
hardly any difference, so I don't think we need to fight this one
out
and can just go with the cleanest code :)
On 16 February 2018 at 11:22, sebb <seb...@gmail.com> wrote:
On 16 February 2018 at 10:01, Stian Soiland-Reyes
<st...@apache.org> wrote:
I agree in general for .clone() on objects - and I'm trying to
move
away from using .clone() in that Commons RDF fluent
mutable/immutable
builder (See COMMONSRDF-49).
But this is an Object[] - a native type.
Huh?
The code says String[], which is native as it's final.
Object[] may not be native
I must admit I am not sure what is really the preferred way - I
thought .clone() is best as the VM can copy an array natively
however
it wants, while with Arrays.copyOf it might have to check if
length is
the same before doing anything clever.
In the specific case of String[] it might be OK to use clone, but
in
general it's better to use the proper JVM methods and not try to
second-guess how efficient they are.
On 13 February 2018 at 18:58, Gilles
<gil...@harfang.homelinux.org> wrote:
On Tue, 13 Feb 2018 18:40:13 +0000, sebb wrote:
On 13 February 2018 at 09:31, Gilles
<gil...@harfang.homelinux.org> wrote:
On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), st...@apache.org
wrote:
Repository: commons-csv
Updated Branches:
refs/heads/CSV-216 637ad2d7a -> f66a83901
CSV-216: Avoid Arrays.copyOf()
Why?
Agreed
as .clone() will do
We should rather avoid using "clone()".
Again: why?
There are many discussions about this topic on the web.[1]
My point is that using "clone()" is not good advice.
Gilles
[1] E.g. http://vojtechruzicka.com/java-cloning-problems/
It's not clear why Arrays.copyOf(0 is considered bad, nor why
clone()
is considered bad.
Gilles
-- at least until someone tries to do
.withValue(x) in an out-of-range column
Project:
http://git-wip-us.apache.org/repos/asf/commons-csv/repo
Commit:
http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
Tree:
http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
Diff:
http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
Branch: refs/heads/CSV-216
Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
Parents: 637ad2d
Author: Stian Soiland-Reyes <st...@apache.org>
Authored: Fri Feb 9 16:49:51 2018 +0000
Committer: Stian Soiland-Reyes <st...@apache.org>
Committed: Tue Feb 13 00:14:52 2018 +0000
----------------------------------------------------------------------
src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
----------------------------------------------------------------------
diff --git
a/src/main/java/org/apache/commons/csv/CSVRecord.java
b/src/main/java/org/apache/commons/csv/CSVRecord.java
index 979119f..2be5c49 100644
--- a/src/main/java/org/apache/commons/csv/CSVRecord.java
+++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
@@ -199,7 +199,7 @@ public class CSVRecord implements
Serializable,
Iterable<String> {
public final CSVRecord immutable() {
if (isMutable()) {
// Subclass is probably CSVMutableRecord,
freeze values
- String[] frozenValue = Arrays.copyOf(values,
values.length);
+ String[] frozenValue = values.clone();
return new CSVRecord(frozenValue, mapping,
comment,
recordNumber, characterPosition);
} else {
return this;
@@ -260,7 +260,7 @@ public class CSVRecord implements
Serializable,
Iterable<String> {
if (isMutable()) {
return this;
}
- String[] newValues = Arrays.copyOf(values,
values.length);
+ String[] newValues = values.clone();
return new CSVMutableRecord(newValues, mapping,
comment,
recordNumber, characterPosition);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
