[ 
https://issues.apache.org/jira/browse/DERBY-6391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15401200#comment-15401200
 ] 

Bryan Pendleton commented on DERBY-6391:
----------------------------------------

I don't think your proposed patch is wrong; I think the new behavior would be 
correct.

I think the question we have to study is this:
{quote}
Is the new code either more efficient or easier to read than the existing code?
{quote}

I think that the title and description of this JIRA are rather mis-leading. The 
point of 
the auto-boxing features of Java isn't about removing unneeded object creation; 
it's about writing less code, and easier-to-read code.

As Knut Anders described above in 
https://issues.apache.org/jira/browse/DERBY-6391?focusedCommentId=13804029&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13804029
the newer Java compilers can automatically write certain simple
code patterns that we formerly had to write by hand.

An example of this is in the StoredPage.java code that Mike included in the 
JIRA description.

That code used to be written as:
{code}
                throw StandardException.newException(
                    SQLState.FILE_BAD_CHECKSUM,
                    id,
                    new Long(checksum.getValue()), 
                    new Long(onDiskChecksum), 
                    pagedataToHexDump(pageData));
{code}

In revision 1742858, Rick changed that code to:
{code}
                throw StandardException.newException(
                    SQLState.FILE_BAD_CHECKSUM,
                    id,
                    checksum.getValue(),
                    onDiskChecksum,
                    pagedataToHexDump(pageData));
{code}

As I understand it, nothing actually changed about the Java bytecode that is 
written to
StoredPage.class due to this change, because the Java compiler now detects that 
we
are passing a value of type 'long' to a method which expects a value of type 
'Object',
and *automatically* generates the call to the "new Long(long)" constructor.

So we aren't actually removing any object creation here (except in the very 
special
case where the argument is in the range -127..127, which isn't true for page 
checksums).

BUT, the advantage is that the new code is cleaner and easier to read without 
the
"new Long()" syntax getting in the way.

And, as Rick noticed, in Java 9 our code actually gets a *warning* from the 
compiler if
we put the call to "new Long()" in the code ourselves, so it's nice to get rid 
of those
warnings and let the compiler do the code generation for us.

Now, that case that you found seems very similar:
{code}
             throw StandardException.newException(
                     SQLState.LANG_INVALID_ESCAPE_CHARACTER,
                    new String(escapeCharArray));
{code}

Here, we are passing a 'char []' object to a method which expects 'Object'.

It would be nice if the Java compiler automatically detected this case, like it
does with 'long' and 'int' and 'short' and 'float' and 'double' and some other 
cases.

But, as Rick pointed out in our discussion over on DERBY-6856, 'char []'
doesn't work that way, and the auto-boxing features of the Java compiler
do *NOT* automatically generate the invocation of the 'new String(char[])'
constructor.

So, it's clear that we don't get to remove the 'new String(char[])' constructor
entirely; all we could do is to replace it with the 'String.valueOf()' static
method instead.

With either syntax, we are creating a new String object which will be
garbage collected. The only question is whether one way is easier-to-read
than the other way.

Frankly, I'm not actually sure why Java provides both ways to make a new String
from a 'char []' variable. I have this vague memory that this may date back to
a time very early in Java, perhaps even prior to Java version 1.0, when String
objects were not actually immutable. I think this is also why there is the 
static method
{code}
String.copyValueOf( char [] )
{code}

Nowadays, though, I believe that 'new String(char [])', 
'String.valueOf(char[])',
and 'String.copyValueOf(char[])' are all identical in their result.

So, to summarize, I think that we can leave the 'new String(char[])' calls in 
the
existing code alone, because I don't think we'd see much benefit by changing
them to 'String.valueOf(char[])'.

Does this make sense?

thanks,

bryan


> remove unneeded object creation in newException() calls in releases > 10.10
> ---------------------------------------------------------------------------
>
>                 Key: DERBY-6391
>                 URL: https://issues.apache.org/jira/browse/DERBY-6391
>             Project: Derby
>          Issue Type: Improvement
>          Components: SQL, Store
>    Affects Versions: 10.11.1.1
>            Reporter: Mike Matrigali
>            Assignee: Danoja Dias
>              Labels: derby_backport_reject_10_10
>         Attachments: Derby-6391.diff
>
>
> In releases after 10.10 the code has been converted to use new 
> java language features.  One of the benefits I just noticed is that
> arguments to StandardException.newException() no longer have
> to be Objects.  I believe this is due to reimplementation using varargs.
> As an example old code use to have to be written as:
> throw StandardException.newException(
>                     SQLState.FILE_BAD_CHECKSUM,
>                     id,
>                     new Long(checksum.getValue()),
>                     new Long(onDiskChecksum),
>                     pagedataToHexDump(pageData));
> The only reason for the new Long() calls was to make them Objects so
> that the call would match up to a hard coded N Object arg version of
> the newException call.  I believe these conversions to Objects are no
> longer needed (but formatting of the args might change).
> There may be code size savings to be had by doing this code
> rototil.
> Anyone see a downside to changing the code in this way?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to