RangeIntGenerator does not split Range properly

[Niklas974]

The RangeIntGenerator is supposed to split the range in to su-ranges for every thread, and return different ints to different threads. So for a range of 100, used by 10 threads, the first thread should get ints 1-10, the second thread should get 11-20, etc…

As there is only one offset pointer (cursor) implemented, this can not work. If threads one, two, three ask in this order, thread one will get "1", thread two will get "12", thread 3 will get "23", etc…

I think the Class can not be implemented properly with the given signature of next(), as one can not be sure that the amount of threads won't change. If the amount of threads given by "int all" changes, all the splitting of the range breaks apart.

Also synchronizing does next() not give best performance. It would be better to use AtomicInt for the pointer (cursor).

[ywang19]

below commit is trying to fix it, could you verify it?
8714f79

[Niklas974]

I fixed your test in this commit: Niklas974@a4a6289

You used a separate RangeIntGenerator for each test, where as in productive use only one RangeIntGenerator is used for all the threads.
If you can be sure that the amount of threads won't change, you could implement this class with a map of AtomicInts, if the amount of threads (int all) does change (ramp-up, ramp-down???), the way I see it this class can not be implemented correctly.

[ywang19]

That’s the way cosbench works, each worker will get its segment, when one hits its segment limit, it will wrap back.

From: Niklas974 [mailto:notifications@github.com]
Sent: Monday, July 01, 2013 6:56 PM
To: intel-cloud/cosbench
Cc: Wang, Yaguang
Subject: Re: [cosbench] RangeIntGenerator does not split Range properly (#64)

I fixed your test in this commit: Niklas974/cosbench@a4a6289Niklas974@a4a6289

You used a separate RangeIntGenerator for each test, where as in productive use only one RangeIntGenerator is used for all the threads.
If you can be sure that the amount of threads won't change, you could implement this class with a map of AtomicInts, if the amount of threads (int all) does change (ramp-up, ramp-down???), the way I see it this class can not be implemented correctly.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/64#issuecomment-20274962.

[Niklas974]

Yes, but with 5 workers on a range from 50 to 100, if each asks for 10 values, none of them should get values twice.
As you only have one cursor, you can only remember ONE offset, but for 5 threads you'd need to remember 5 of them…

[ywang19]

will verify to decide if need enhancement. BTW, should consider how to clarify the difference between range and sequential selector.

[ywang19]

using atomic structure in Range selector is insufficient to avoid race condition, synchronization primitive will be added in object picker to coordinate container and object generators.

[ywang19]

see commit db46e03