Reference semantics in std.random #893

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@monarchdodra
Collaborator

This development addresses http://forum.dlang.org/thread/oiczxzkzxketxitncghl@forum.dlang.org which has raised bugs, such as Issue 8247 ( http://d.puremagic.com/issues/show_bug.cgi?id=8247 )

Basically, it gives all PRNG reference semantics: Cheap copy (as a reminder, a Merseen is a 1.5 KB object...), but more importantly: No hidden save on copy.

Using RefRange was not an option, because:

  1. It requires explicit usage
  2. It does not lazily initialize.

About the development:

The current "stack" based implementation where left as-is, but made private. I provided some new PRNGs, which simply wrap a pointer to said implementation, and forward all calls correctly. This was done "by hand" (as opposed to using a mixin), so that everything could be documented. The "stack based" implementations were purposefully kept as is because:

  1. Less impact
  2. We can imagine they will one day also be kept inside aggressive containers.

Regarding the discussion in the thread, I did not provide ANY new functionality, workaround or feature for accessing the stack implementation. I fear adding new functionality now would bite us in the ass if we decided to move std.random in some other direction later.

Also in this Pull Request:

  • I added an entire StdDdoc section to provide a "unified" documentation for all PRNGs. I think there are 2 kinds of documentations: The one relative to PRNGs in the generic sense, and the one relative to a specific implementation. This is the generic documentation.
  • Debug traces. Helps a lot.
  • safe + nothrow + const where possible.

NOT in this pull request:

  • I think saving a PRNG should be deprecated. I think it is OK to dupe one if the developer explicitly wants it, but it really doesn't make sense for a PRNG to be a ForwardRange (as opposed to a simple InputRange).
@dsimcha
Collaborator
dsimcha commented Oct 27, 2012

I'm slightly concerned that some existing code might rely on the implicit save to e.g. generate the same random sequence multiple times. I've written some like that, though it was throwaway code, so I don't care too much if it bit-rots. Then again, I don't see any reasonable alternative to breaking this code. Thoughts?

@monarchdodra
Collaborator

I'm slightly concerned that some existing code might rely on the implicit save to e.g. generate the same random sequence multiple times. I've written some like that, though it was throwaway code, so I don't care too much if it bit-rots. Then again, I don't see any reasonable alternative to breaking this code. Thoughts?

I think allowing a PRNG to implicitly save is just asking for trouble. The probability you'd accidentally generate the same "random" sequence twice are IMO, too high. Not to mention the cost. Every time you "implicitly" save, you are allocating a new 1.5KB object on the stack. Implicitly save a few times too many, and it's overflow.

Regarding breaking existing code... I'll admit I hadn't thought that someone might purposefully save via implicit.

I think breaking such (unsafe (imo)) code is a necessary evil. But I do wonder how much code there is out there, where an explicit save is required, but implemented via an implicit save...

@andralex
Member
andralex commented Nov 4, 2012

I'd say let's go with std.random2 for this big change of semantics. There's no way we can afford breaking this.

@monarchdodra
Collaborator

Closing per @andralex suggestion.

@monarchdodra monarchdodra deleted the monarchdodra:refRandom branch Apr 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment