commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (TEXT-84) RandomStringGenerator claims to be immutable, but isn't
Date Tue, 30 May 2017 11:02:04 GMT

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

Gilles commented on TEXT-84:
----------------------------

bq. I suppose you could run some tests to prove your assumptions around performance?

First, I'm not going to spend time on this, given the experience I had with trying to get
(constructive) feedback about the subject when I was redesigning/implementing the RNG classes
originally in "Commons Math" (see ML archive sometime around December 2015).

Second, depending on the intended use-cases, it might not even be necessary to go beyond a
crude estimation, by looking at the tables in the ["Commons RNG" userguide|http://commons.apache.org/proper/commons-rng/userguide/rng.html#a4._Performance]:
For generating {{int}} values (the case at hand for this discussion), the performance range
is 44% to 124% that of {{java.util.Random}} (which is *synchronized*).

The question is whether "Commons Text" wants to make an assumption about usage.
If not, it should also not assume that the RNG is thread-safe because it will necessarily
hamper performance (alternatively, to have a non-synchronized, yet thread-safe RNG, it should
be ["splittable"|https://docs.oracle.com/javase/8/docs/api/java/util/SplittableRandom.html#split--]
but it will seriously complicate the API).

Currently, the default behaviour assumes that performance is not crucial since it uses {{java.concurrent.ThreadLocalRandom}}
(which *extends*{{java.util.Random}}).
The non-default behaviour explicitly allows better performance, at the "cost" of leaving parallelism
issues to the caller.

So an option is to leave the code as is, only changing the Javadoc to read:
-----
RandomStringGenerator instances are thread-safe only if using the default RNG.
If a custom RNG is set by calling the method [usingRandom|http://commons.apache.org/proper/commons-text/javadocs/api-release/org/apache/commons/text/RandomStringGenerator.Builder.html#usingRandom-org.apache.commons.text.TextRandomProvider-],
thread-safety must be ensured externally.
-----

bq. Supposing it existed, how would you propose to use ThreadLocalRandom with RandomStringGenerator?

It would still be up to the user to call {{usingRandom(RandomSource.current())}}, similar
to what is done with the {{ThreadLocalRandom}} class of the JDK.


> RandomStringGenerator claims to be immutable, but isn't
> -------------------------------------------------------
>
>                 Key: TEXT-84
>                 URL: https://issues.apache.org/jira/browse/TEXT-84
>             Project: Commons Text
>          Issue Type: Bug
>    Affects Versions: 1.1
>            Reporter: Duncan Jones
>
> {{RandomStringGenerator}} claims to be immutable in the Javadocs, however it accepts
a {{TextRandomProvider}} object through the builder pattern. This object may altered by external
code, thus breaking the immutability claim of our generator.
> A possible solution is to adjust the documentation for {{TextRandomProvider}} and require
implementations to be immutable. Alternatively, we can relax the documentation in {{RandomStringGenerator}}
to remove the immutability claim or state that the mutability is linked to the mutability
of the random source (when provided).
> I think we will have to do the latter, since the former would forbid callers from supplying
a {{UniformRandomProvider}} instance as suggested in the documentation. Thoughts welcome.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message