Skip to content
This repository has been archived by the owner on Nov 16, 2021. It is now read-only.

[RFC 216/ PushStream] Inconsistent time unit of PushbackPolicyOption.getPolicy() between JavaDoc and implementation #40

Open
bjhargrave opened this issue May 17, 2019 · 4 comments
Labels
publiccomment Public comment

Comments

@bjhargrave
Copy link
Member

Original bug ID: BZ#231
From: Clément Delgrange <cl.delgrange@protonmail.com>
Reported version: R7

@bjhargrave
Copy link
Member Author

Comment author: Clément Delgrange <cl.delgrange@protonmail.com>

Overview:

The JavaDoc of the PushbackPolicyOption.getPolicy method indicates that the time unit is nanoseconds, however, in the reference implementation org.osgi:org.osgi.util.pushstream:1.0.0, when a tasks is scheduled (eg; BufferedPushStreamImpl.startWorker), the time unit used is miliseconds.

The time unit of the time parameter is also missing in the JavaDoc of BufferBuilder.withPushbackPolicy(PushbackPolicyOption pushbackPolicyOption, long time)

Actual Results:

The user cannot decide which of the specification or the implementation is right. This may lead to bugs in the client code.

Expected Results:

Consistence between JavaDoc and implementation.

@bjhargrave
Copy link
Member Author

Comment author: Clément Delgrange <cl.delgrange@protonmail.com>

Not sure if it is when a task is scheduled, but when a default PushBackPolicy is set in PushStreamProvider.createStream:

if (pushbackPolicy == null) {
   pushbackPolicy = LINEAR.getPolicy(1000);
}

The provided time does not seem to be nanosecondes.

@bjhargrave
Copy link
Member Author

Comment author: Castro B <castro8583bennett@gmx.com>

(In reply to Clément Delgrange from comment BZ#1)

The provided time does not seem to be nanosecondes.

Hi Clement i really didn't get what you mean,

Castro B,
https://sparpedia.no

@bjhargrave
Copy link
Member Author

Comment author: Clément Delgrange <cl.delgrange@protonmail.com>

Hi,

Sorry, I will try to be more explicit.

First, the abstract method PushbackPolicyOption.getPolicy(long value) indicates in its JavaDoc that this method create an instance configured with a base back pressure time in nanoseconds. So, I have assumed that the value parameter must be a time in nanoseconds. As well as for each implementation LINEAR, ON_FULL_EXPONENTIAL, FIXED, ...

Then, from a Tim Ward's answer on stackoverflow (https://stackoverflow.com/questions/53692861/osgi-pushstream-is-slow), he said:

When you create a stream using psp.createStream(source) it is set up with a buffer of 32 elements and a linear back pressure policy based on the size of the buffer, returning one second when full...

Now, when you look at the implementation of PushStreamProvider.createStream:

return createStream(eventSource, 1, null, null,
				new ArrayBlockingQueue<>(32),
				FAIL.getPolicy(), LINEAR.getPolicy(1000));

if the value provided to LINEAR.getPolicy is a time in milliseconds, when the buffer is full we will get a back pressure of 1 second ( 1000*32/(32+0)=1000ms=1s ) as Tim Ward said but not if it is nanoseconds.

Am I right, or I miss something?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
publiccomment Public comment
Projects
None yet
Development

No branches or pull requests

1 participant