Am 10.01.2013 03:11, schrieb Vladimir Kozlov:
On 1/9/13 5:11 PM, Vitaly Davidovich wrote:
One could write this as:
boolean overflow = false;
if (len > (dl - dp))
{
   overflow = true;
    len = dl - dp;
}

One would hope jit can do this automatically and also CSE away the dl -
dp bit. :)

Yes, JIT can convert Ulf's code into above code - SplitIf optimization in C2.

Yes, this I was assuming too. But I think, the correct code would be:
Alternative (3):
    boolean overflow;
    if (len > (dl - dp)) {
        overflow = true;
        len = dl - dp;
    } else
        overflow = false;
This would avoid first assigning overflow = false and later reverting it to 
overflow = true;
...or is JIT able to optimize this?

But it could be complicated by surrounding code.

Could one check this by hsdis ?

Anyway I suspect if JIT will optimise this snippet in a reasonable time, because it is only executed once in encodeArrayLoop(), + the compile threshold is high, as method encodeArrayLoop() is long. So maybe we should focus on the best performance in interpreter for the 3 alternatives.
My assumption, the following would win, as it avoids to calculate dl - dp 2 
times:
Alternative (4):
    int slen = sl - sp;
    int dlen = dl - dp;
    boolean overflow;
    if (slen > dlen) {
        overflow = true;
        slen = dlen;
    } else
        overflow = false;
... but maybe Alternative (5) is not far from that:
    int slen = sl - sp;
    int dlen = dl - dp;
    boolean overflow = slen > dlen;
    slen ? overflow = dlen : slen;

In case, if JIT comes in effect, both should optimize optimal.

Looking at the calculation of slen/dlen:
    int sp = src.arrayOffset() + src.position();
    int sl = src.arrayOffset() + src.limit();
    assert (sp <= sl);
    sp = (sp <= sl ? sp : sl);
    int slen = sl - sp;
My suggestion:
int slen = src.remaining(); // should never be negative, so above 2 time (sp <= sl) are superfluous!
Additionally calculation of sl could be faster:
    int sl = sp + src.remaining();
which could be again faster and is equivalent to:
    int sl = sp + slen;
if slen is anyway calculated before.

Same for dlen, with on top, that dl is never used later 8-) , so could be 
completely dropped.

About the naming:
According to sp, sl, dp, dl I would prefer: sr, dr over (x)len.

About overflow:
There was a question some posts above, if it is necessary at all. Can one 
please answer?

The question is, if other error results have higher priority over 
CoderResult.OVERFLOW.

-Ulf


And generating only one dl-dp is regular optimization which JIT does.

Thanks,
Vladimir


Sent from my phone

On Jan 9, 2013 7:39 PM, "Vladimir Kozlov" <vladimir.koz...@oracle.com
<mailto:vladimir.koz...@oracle.com>> wrote:

     From JIT compiler view current code is better since it has only one
    compare. Your code has two compares: one to calculate "overflow" and
    an other (depended on first) to calculate "len".

    Thanks,
    Vladimir

    On 1/9/13 3:57 PM, Ulf Zibis wrote:

        Another little simplification:
            179             boolean overflow = sr > dr;
            180             sr = overflow ? dr : sr;
        or in your existing logic:
           178             int len = sl - sp;
           179             boolean overflow = len > (dl - dp);
           180             len = overflow ? dl - dp : len;
        (len is equivalent to sr)

        -Ulf

        Am 09.01.2013 19:03, schrieb Vladimir Kozlov:

            Ulf,

            Thank you for this suggestion but I would like to keep
            surrounding
            code intact. I will rename "overflowflag" to "overflow". It
            is used to
            indicate that we should return CoderResult.OVERFLOW result.

            Thanks,
            Vladimir

            On 1/9/13 3:58 AM, Ulf Zibis wrote:

                Am 09.01.2013 01:10, schrieb Vitaly Davidovich:

                    On Jan 8, 2013 6:18 PM, "Vladimir Kozlov"
                    <vladimir.koz...@oracle.com
<mailto:vladimir.koz...@oracle.com>>
                    wrote:

http://cr.openjdk.java.net/~**__kvn/6896617_jdk/webrev
<http://cr.openjdk.java.net/~**kvn/6896617_jdk/webrev><http://__cr.openjdk.java.net/~kvn/__6896617_jdk/webrev
<http://cr.openjdk.java.net/~kvn/6896617_jdk/webrev>>



                Another tweak:
                   168             char[] sa = src.array();
                   169             int sp = src.arrayOffset() +
                src.position();
                   170             int sr = src.remaining();
                   171             int sl = sp + sr;
                   172             assert (sp <= sl); // superfluous, sr
                is always >= 0
                   173             sp = (sp <= sl ? sp : sl); //
                superfluous "
                   174             byte[] da = dst.array();
                   175             int dp = dst.arrayOffset() +
                dst.position();
                   170             int dr = dst.remaining();
                   176             int dl = dp + dr;
                   177             assert (dp <= dl); // superfluous   "
                   178             dp = (dp <= dl ? dp : dl); //
                superfluous "
                   179             boolean overflow = false;
                   180             if (sr > dr) {
                   181                 sr = dr;
                   182                 overflow = true;
                   183             }

                Why you called it "overflowflag", in that way, you could
                name each
                variable "myvaluevariable" or "myvaluefield" ;-)


                -Ulf





Reply via email to