New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 675813 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

When mathrand is not installed in a Context, each Get call instantiates a new one!

Project Member Reported by d...@chromium.org, Dec 20 2016

Issue description

This is the case for AppEngine instances. Currently, each mathrand call (datastore cache filter) will cause a new Rand to be initialized. This ruins randomness and also costs a lot.

We should replace this with a global rand instance now that we have locking.
 

Comment 1 by d...@chromium.org, Dec 20 2016

Cc: benhenry@chromium.org
(Making a bug b/c benhenry@ asked nicely).
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/f469f2540cad71872c674950f530bdbc5de13e02

commit f469f2540cad71872c674950f530bdbc5de13e02
Author: dnj <dnj@chromium.org>
Date: Tue Dec 20 02:47:13 2016

Use global rand.Rand instance in mathrand.

By default, mathrand will use a new *rand.Rand if none is installed in
the Context. This leads to a very bad performance case if the user
forgets to install an instance in the Context!

Now that we have locking, use a single global rand.Rand instance. The
user can always change this if they want. Update all of the global
functons to use our global interface instead of the rand.Rand package
one.

Additionally, as an optimization, remove "defer" calls from the Locking
wrapper. This speeds up operations by a factor of 3x.

BUG= chromium:675813 
TEST=None

Review-Url: https://codereview.chromium.org/2588213002

[modify] https://crrev.com/f469f2540cad71872c674950f530bdbc5de13e02/appengine/gaemiddleware/context.go
[modify] https://crrev.com/f469f2540cad71872c674950f530bdbc5de13e02/common/data/rand/mathrand/impl.go
[modify] https://crrev.com/f469f2540cad71872c674950f530bdbc5de13e02/common/data/rand/mathrand/mathrand.go
[modify] https://crrev.com/f469f2540cad71872c674950f530bdbc5de13e02/common/data/rand/mathrand/mathrand_test.go

Comment 3 by d...@chromium.org, Dec 20 2016

Status: Fixed (was: Untriaged)

Sign in to add a comment