FAQ
Hello Brad,
So if GOMAXPROCS changes at runtime, the pollServer() accessor can
lose its *pollServer?
Yes and no. A same `*netFD' can indeed get buried in different
pollServers over the time, but we don't really care as long as the
selected pollServer is maintained for each single {wait,unblock}
sequence, which always happens in e.g `fd.pollServer().WaitWrite(fd)'.
I/O can't also happen concurrently thanks to rio/wio mutexes.
All the tests pass, an aborting Close should however be racy if
GOMAXPROCS change in the between.
Why not just store the *pollServer in the *netFD?
Indeed, seems to be the safest/easiest way to maintain hard coherency at
the *netFD level. Thanks.
GOMAXPROCS changing at runtime used to sound crazy, but it's not
uncommon now
It's exactly the purpose of the last patch set, the (quite used)
benchmark framework relies on that feature too.

Sebastien

http://codereview.appspot.com/6496054/

Search Discussions

  • Sebastien Paolacci at Sep 20, 2012 at 11:03 am
    I eventually manage to make `TestTCPListenClose' fail by injecting a
    dummy `rand.Int() % pollN' selection code in `pollserver()'. Even if it
    doesn't make so much sense, it should have worked.

    CL updated with Brad's suggestion, please take another look.

    Thanks,
    Sebastien

    https://codereview.appspot.com/6496054/
  • Dave at Sep 20, 2012 at 11:12 am
    Thank you for your proposal, but I remain uncomfortable with it. In
    return for additional scalability, a lot more code has been added in the
    hot path of newFD which will impact the latency of every accept and dial
    operation. I am concerned that this is a price that all users of net
    have to pay even if they are not interested in handling tens of
    thousands of active connections.


    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go
    File src/pkg/net/fd_unix.go (right):

    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go#newcode285
    src/pkg/net/fd_unix.go:285: panic(err)
    if any error from newPollServer is a panic, newPollServer() itself
    should panic.

    https://codereview.appspot.com/6496054/
  • Sebastien Paolacci at Sep 20, 2012 at 2:16 pm
    Hello Dave,

    I'm not sure I really got your point. It indeed adds few lines of code
    in the "hot" `newFD()' (not in read and/or write), but if you look at
    them you'll find:
    - an access to an already burned `Once' (that was already present),
    - an additional {modulo + test + two slice lookups} sequence.

    Given that anyway going for few syscalls, two/three mutexes, a
    sleep/wake-up sequence, and hopefully few real bytes of transfer, I
    can't see that as a major latency impact. Benchmarks, given that few
    percents are really within the confidence interval for that kind of run,
    seems decent (to me) even in the cpu=1 situation.

    On the other side, handling few tens of thousands of active connections
    is honestly not a corner-case.

    Cheers,
    Sebastien

    http://codereview.appspot.com/6496054/
  • Bradfitz at Sep 20, 2012 at 6:43 pm
    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go
    File src/pkg/net/fd_unix.go (right):

    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go#newcode308
    src/pkg/net/fd_unix.go:308: startServersOnce[k]()
    this line may crash if GOMAXPROCS has increased since the size it was
    during init(), when startServersOnce was initialized.

    https://codereview.appspot.com/6496054/
  • Bradfitz at Sep 20, 2012 at 7:18 pm
    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go
    File src/pkg/net/fd_unix.go (right):

    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go#newcode308
    src/pkg/net/fd_unix.go:308: startServersOnce[k]()
    On 2012/09/20 19:15:11, dvyukov wrote:
    On 2012/09/20 18:43:18, bradfitz wrote:
    this line may crash if GOMAXPROCS has increased since the size it
    was during
    init(), when startServersOnce was initialized.
    init() does not call GOMAXPROCS, it calls NumCPU()
    sorry, I see. Teaches me to read code on a bus.

    https://codereview.appspot.com/6496054/
  • Dvyukov at Sep 20, 2012 at 7:22 pm
    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go
    File src/pkg/net/fd_unix.go (right):

    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go#newcode303
    src/pkg/net/fd_unix.go:303: pollN := runtime.GOMAXPROCS(0)
    Please move this out of newFD().
    This change addresses inefficiency in current pollserver impl. I
    anticipate that pollserver impl will not only change in future, but also
    diverge for different OSes. The netFD part of this file implements
    generic functionality that won't diverge, so please split it from the
    partitioning logic (most OSes are perfectly able to implement a scalable
    pollserver, and so won't need it).

    https://codereview.appspot.com/6496054/
  • Dvyukov at Sep 20, 2012 at 8:18 pm
    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go
    File src/pkg/net/fd_unix.go (right):

    https://codereview.appspot.com/6496054/diff/23001/src/pkg/net/fd_unix.go#newcode308
    src/pkg/net/fd_unix.go:308: startServersOnce[k]()
    On 2012/09/20 18:43:18, bradfitz wrote:
    this line may crash if GOMAXPROCS has increased since the size it was during
    init(), when startServersOnce was initialized.
    init() does not call GOMAXPROCS, it calls NumCPU()

    https://codereview.appspot.com/6496054/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedSep 20, '12 at 8:28a
activeSep 20, '12 at 8:18p
posts8
users4
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase