Re: websockets

[Pavel Rappo]

> On 12 Feb 2018, at 19:29, Chuck Davis  wrote:
> 
> 
> 
> There is never a way to tell how much data may be
> returned.
> 

Reactive Streams speak in terms of numbers of invocations to the onNext method.
Not the amount of data. (Please disregard if that's not what you meant.)

> 
> 
> I am under the impression that this new API requires the developer to
> buffer the messages until all the data is complete. As I recall (it's
> been a few months since I implemented) decoders handled that in the
> old API and if I am correct that is going to put a lot more work on
> the plate of developers.  If my understanding is correct I'd suggest
> listeners should do the buffering and deliver a complete message.  If
> it's written once in the implementation it would save thousands of
> developer hours recreating the same wheel.  At least this approach
> makes sense in my use case.  Partial pieces of serialized objects are
> useless.
> 

Different developers have different requirements. Some would benefit from having
the ability to receive partial messages. If this API only provided whole message
receiving, it would not be possible to satisfy those developers.
If in your case it makes no sense to use partial messages, you can still use the
API. It will only require some extra work though. In the simplest case the
amount of work is not that big really:

   @Override
   public CompletionStage onText(WebSocket webSocket,
CharSequence message,
WebSocket.MessagePart part) {
   webSocket.request(1);
   String str = null;
   switch (part) {
   case FIRST:
   case PART:
   text.append(message);
   return null;
   case LAST:
   text.append(message);
   str = text.toString();
   text.setLength(0);
   break;
   case WHOLE:
   str = message.toString();
   break;
   }
   processWholeText(str);
   return null;
   }

   private void processWholeText(String string) {
   // -- your code here --
   }

However, with this approach you will need to go the extra mile if you ever need
to talk to WebSocket.request outside of the context of the associated listener.
Because the WebSocket client will continue to speak in terms of the number of
unspecified (could be whole, could be partial) messages.

We might provide this "buffering" ability internally in WebSocket in the future.
One option would be to add an intermediate method to the WebSocket.Builder such
that this method would give an extra guarantee for the attached listener:
 
   Builder receiveWholeMessages(boolean whole)

Another option would be to provide a slightly different listener that would
reflect this behaviour in a more type-safe manner (i.e. the listener won't have
those extra MessagePart arguments in the onText and onBinary methods):

   CompletableFuture buildAsync(URI uri, WholeListener listener)

And these are probably not the only possible options.

It does however seem that optimization-wise there is not much to be gained from
doing this. As different parts of a whole message (i.e. payloads of WebSocket
frames) do not occupy contiguous parts of the underlying memory. In order to be
concatenated, payload pieces would need to be cut out of their frames first.
Which means copying. In this context it might have been better if we decided to
use ByteBuffer[] instead of ByteBuffer in the first place. But that seemed like
an overkill back then.

> 
> Can you expound a little more on this:  "Finally, each of the
> listener's methods asks the user to signal when the user has processed
> the message it bears [2]."  All I see is that the method returns a
> CompletionStage which triggers a procession of CompletionStage objects
> (not sure yet how this ends except by returning a null?) and I don't
> see in the docs how this signals the listener except that the method
> exits and may be called again.  I'm sorry to be so dense here.
> 

Let me explain this the following way. With WebSocket there is an exchange of
messages. You create a new message for the WebSocket client and tell it to send
the message by calling one of the send methods. You'd probably like to know
though when the WebSocket client has actually processed the message. That's
where a CompletableFuture returned from the method comes in handy. When the
WebSocket client completes this future, you know the message has been processed.

Now consider that when the WebSocket client receives a message it calls one of
the receive methods on you. The WebSocket client needs you to tell it when you
finished processing the message it gave you. You do this by returning a
CompletionStage from the method. Once this stage reaches the WebSocket client,
the client then attaches dependant actions on this stage internally. But that's
details of the implementation, the point is the scenario is really symmetric.

Now, in th

Re: [11] RFR: 8197849: Misc improvements to URL parsing

[Claes Redestad]

Thanks for reviewing, Roger

/Claes

On 2018-02-14 17:50, Roger Riggs wrote:

Hi Claes,

Looks fine,

Roger


On 2/14/2018 11:36 AM, Claes Redestad wrote:
Right, I expanded the range as much as possible given the constraint 
provided by L_ENCODED minus '/'. I will think of a better comment to 
this effect.


/Claes

Daniel Fuchs  skrev: (14 februari 2018 
17:25:20 CET)


    Hi Claes,

    Your proposed changes to ParseUtil look reasonable
    to me, though I had to carefully compare the characters
    in the range (c >= '&' && c <= ':') with the
    L_ENCODED / H_ENCODED masks to convince myself
    that there was no behavior change to
    ParseUtil::firstEncodeIndex.

    I wonder whether this would deserve some additional
    comment - though I'm not sure how it could be formulated.

    Given the sensitivity of the impacted code maybe it would
    be prudent to wait for a second review before pushing.

    best regards,

    -- daniel

    On 14/02/2018 15:30, Claes Redestad wrote:

    Hi, as a means to improve startup in some applications, please
    review this set of small improvements to improve both
    interpreted and compiled performance of creating and handling
    certain jar URLs. Some of the changes in
    sun.net.www.ParseUtil::encodePath have a small, positive
    effect when dealing with other types of path resources. Bug:
    https://bugs.openjdk.java.net/browse/JDK-8197849 Webrev:
    http://cr.openjdk.java.net/~redestad/8197849/jdk.00/
 This
    shaves off a percent or so of the total bytecode execution in
    a few of our startup tests: - ParseUtil::encodePath cost is
    reduced ~20% during startup, averaging ~10-15% faster for
    typical inputs after JIT. Weird examples like paths only
    consisting of slashes and dots can be seen to take a small hit
    due to not getting special treatment. -
    ParseUtil::canonizeString cost on startup reduced by 50% (~15%
    improvement after JIT) for typical inputs by adding a test to
    return directly if there's no need to "canonize" the string
    (which is typically always the case for well-formed jar
    files). I added a sanity test to ensure I didn't accidentally
    change semantics of cases that would lead to canonicalization.
    - Removed a couple of unnecessary allocation in
    sun.net.www.protocol.jar.Handler. Maybe there are some good
    reasons not to make ParseUtil a final utility class with only
    static methods and a private constructor, though... Thanks!
    /Claes


--
Sent from my Android device with K-9 Mail. Please excuse my brevity. 

Re: [11] RFR: 8197849: Misc improvements to URL parsing

[Roger Riggs]

Hi Claes,

Looks fine,

Roger


On 2/14/2018 11:36 AM, Claes Redestad wrote:
Right, I expanded the range as much as possible given the constraint 
provided by L_ENCODED minus '/'. I will think of a better comment to 
this effect.


/Claes

Daniel Fuchs  skrev: (14 februari 2018 
17:25:20 CET)


Hi Claes,

Your proposed changes to ParseUtil look reasonable
to me, though I had to carefully compare the characters
in the range (c >= '&' && c <= ':') with the
L_ENCODED / H_ENCODED masks to convince myself
that there was no behavior change to
ParseUtil::firstEncodeIndex.

I wonder whether this would deserve some additional
comment - though I'm not sure how it could be formulated.

Given the sensitivity of the impacted code maybe it would
be prudent to wait for a second review before pushing.

best regards,

-- daniel

On 14/02/2018 15:30, Claes Redestad wrote:

Hi, as a means to improve startup in some applications, please
review this set of small improvements to improve both
interpreted and compiled performance of creating and handling
certain jar URLs. Some of the changes in
sun.net.www.ParseUtil::encodePath have a small, positive
effect when dealing with other types of path resources. Bug:
https://bugs.openjdk.java.net/browse/JDK-8197849 Webrev:
http://cr.openjdk.java.net/~redestad/8197849/jdk.00/
 This
shaves off a percent or so of the total bytecode execution in
a few of our startup tests: - ParseUtil::encodePath cost is
reduced ~20% during startup, averaging ~10-15% faster for
typical inputs after JIT. Weird examples like paths only
consisting of slashes and dots can be seen to take a small hit
due to not getting special treatment. -
ParseUtil::canonizeString cost on startup reduced by 50% (~15%
improvement after JIT) for typical inputs by adding a test to
return directly if there's no need to "canonize" the string
(which is typically always the case for well-formed jar
files). I added a sanity test to ensure I didn't accidentally
change semantics of cases that would lead to canonicalization.
- Removed a couple of unnecessary allocation in
sun.net.www.protocol.jar.Handler. Maybe there are some good
reasons not to make ParseUtil a final utility class with only
static methods and a private constructor, though... Thanks!
/Claes 




--
Sent from my Android device with K-9 Mail. Please excuse my brevity. 

Re: [11] RFR: 8197849: Misc improvements to URL parsing

[Claes Redestad]

Right, I expanded the range as much as possible given the constraint provided 
by L_ENCODED minus '/'. I will think of a better comment to this effect.

/Claes

Daniel Fuchs  skrev: (14 februari 2018 17:25:20 CET)
>Hi Claes,
>
>Your proposed changes to ParseUtil look reasonable
>to me, though I had to carefully compare the characters
>in the range (c >= '&' && c <= ':') with the
>L_ENCODED / H_ENCODED masks to convince myself
>that there was no behavior change to
>ParseUtil::firstEncodeIndex.
>
>I wonder whether this would deserve some additional
>comment - though I'm not sure how it could be formulated.
>
>Given the sensitivity of the impacted code maybe it would
>be prudent to wait for a second review before pushing.
>
>best regards,
>
>-- daniel
>
>On 14/02/2018 15:30, Claes Redestad wrote:
>> Hi,
>> 
>> as a means to improve startup in some applications, please review
>this 
>> set of small improvements to improve both interpreted and compiled 
>> performance of creating and handling certain jar URLs. Some of the 
>> changes in sun.net.www.ParseUtil::encodePath have a small, positive 
>> effect when dealing with other types of path resources.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8197849
>> 
>> Webrev: http://cr.openjdk.java.net/~redestad/8197849/jdk.00/
>> 
>> This shaves off a percent or so of the total bytecode execution in a
>few 
>> of our startup tests:
>> 
>> - ParseUtil::encodePath cost is reduced ~20% during startup,
>averaging 
>> ~10-15% faster for typical inputs after JIT. Weird examples like
>paths 
>> only consisting of slashes and dots can be seen to take a small hit
>due 
>> to not getting special treatment.
>> 
>> - ParseUtil::canonizeString cost on startup reduced by 50% (~15% 
>> improvement after JIT) for typical inputs by adding a test to return 
>> directly if there's no need to "canonize" the string (which is
>typically 
>> always the case for well-formed jar files). I added a sanity test to 
>> ensure I didn't accidentally change semantics of cases that would
>lead 
>> to canonicalization.
>> 
>> - Removed a couple of unnecessary allocation in 
>> sun.net.www.protocol.jar.Handler. Maybe there are some good reasons
>not 
>> to make ParseUtil a final utility class with only static methods and
>a 
>> private constructor, though...
>> 
>> Thanks!
>> 
>> /Claes

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [11] RFR: 8197849: Misc improvements to URL parsing

[Daniel Fuchs]

Hi Claes,

Your proposed changes to ParseUtil look reasonable
to me, though I had to carefully compare the characters
in the range (c >= '&' && c <= ':') with the
L_ENCODED / H_ENCODED masks to convince myself
that there was no behavior change to
ParseUtil::firstEncodeIndex.

I wonder whether this would deserve some additional
comment - though I'm not sure how it could be formulated.

Given the sensitivity of the impacted code maybe it would
be prudent to wait for a second review before pushing.

best regards,

-- daniel

On 14/02/2018 15:30, Claes Redestad wrote:

Hi,

as a means to improve startup in some applications, please review this 
set of small improvements to improve both interpreted and compiled 
performance of creating and handling certain jar URLs. Some of the 
changes in sun.net.www.ParseUtil::encodePath have a small, positive 
effect when dealing with other types of path resources.


Bug: https://bugs.openjdk.java.net/browse/JDK-8197849

Webrev: http://cr.openjdk.java.net/~redestad/8197849/jdk.00/

This shaves off a percent or so of the total bytecode execution in a few 
of our startup tests:


- ParseUtil::encodePath cost is reduced ~20% during startup, averaging 
~10-15% faster for typical inputs after JIT. Weird examples like paths 
only consisting of slashes and dots can be seen to take a small hit due 
to not getting special treatment.


- ParseUtil::canonizeString cost on startup reduced by 50% (~15% 
improvement after JIT) for typical inputs by adding a test to return 
directly if there's no need to "canonize" the string (which is typically 
always the case for well-formed jar files). I added a sanity test to 
ensure I didn't accidentally change semantics of cases that would lead 
to canonicalization.


- Removed a couple of unnecessary allocation in 
sun.net.www.protocol.jar.Handler. Maybe there are some good reasons not 
to make ParseUtil a final utility class with only static methods and a 
private constructor, though...


Thanks!

/Claes

[11] RFR: 8197849: Misc improvements to URL parsing

[Claes Redestad]

Hi,

as a means to improve startup in some applications, please review this 
set of small improvements to improve both interpreted and compiled 
performance of creating and handling certain jar URLs. Some of the 
changes in sun.net.www.ParseUtil::encodePath have a small, positive 
effect when dealing with other types of path resources.


Bug: https://bugs.openjdk.java.net/browse/JDK-8197849

Webrev: http://cr.openjdk.java.net/~redestad/8197849/jdk.00/

This shaves off a percent or so of the total bytecode execution in a few 
of our startup tests:


- ParseUtil::encodePath cost is reduced ~20% during startup, averaging 
~10-15% faster for typical inputs after JIT. Weird examples like paths 
only consisting of slashes and dots can be seen to take a small hit due 
to not getting special treatment.


- ParseUtil::canonizeString cost on startup reduced by 50% (~15% 
improvement after JIT) for typical inputs by adding a test to return 
directly if there's no need to "canonize" the string (which is typically 
always the case for well-formed jar files). I added a sanity test to 
ensure I didn't accidentally change semantics of cases that would lead 
to canonicalization.


- Removed a couple of unnecessary allocation in 
sun.net.www.protocol.jar.Handler. Maybe there are some good reasons not 
to make ParseUtil a final utility class with only static methods and a 
private constructor, though...


Thanks!

/Claes