Skip to content

src: refactor stream callbacks and ownership

Rodrigo Muino Tomonari requested to merge github/fork/addaleax/stream-listener into master

Instead of setting individual callbacks on streams and tracking stream ownership through a boolean consume_ flag, always have one specific listener object in charge of a stream, and call methods on that object rather than generic C-style callbacks.

Benchmark results show no significant changes: 
$ ./node benchmark/compare.js --runs 5 --new ./node --old ./node-master net | Rscript benchmark/compare.R
[00:43:05|% 100| 8/8 files | 10/10 runs | 6/6 configs]: Done
                                                                      improvement confidence     p.value
 net/net-c2s-cork.js dur=5 type="buf" len=1024                           -0.80 %            0.720985414
 net/net-c2s-cork.js dur=5 type="buf" len=128                            -3.50 %            0.278786279
 net/net-c2s-cork.js dur=5 type="buf" len=16                             -4.44 %          * 0.010458284
 net/net-c2s-cork.js dur=5 type="buf" len=32                             -0.51 %            0.445313528
 net/net-c2s-cork.js dur=5 type="buf" len=4                              -1.57 %            0.074816557
 net/net-c2s-cork.js dur=5 type="buf" len=512                            -0.25 %            0.926451422
 net/net-c2s-cork.js dur=5 type="buf" len=64                              1.66 %          * 0.020469582
 net/net-c2s-cork.js dur=5 type="buf" len=8                              -0.18 %            0.739524856
 net/net-c2s.js dur=5 type="asc" len=102400                              -0.22 %            0.904819514
 net/net-c2s.js dur=5 type="asc" len=16777216                             0.34 %            0.862222556
 net/net-c2s.js dur=5 type="buf" len=102400                              -0.45 %            0.755593966
 net/net-c2s.js dur=5 type="buf" len=16777216                             1.87 %            0.477896886
 net/net-c2s.js dur=5 type="utf" len=102400                              -0.30 %            0.572739665
 net/net-c2s.js dur=5 type="utf" len=16777216                             1.18 %            0.369268245
 net/net-pipe.js dur=5 type="asc" len=102400                              1.18 %            0.368102481
 net/net-pipe.js dur=5 type="asc" len=16777216                            0.41 %            0.659646192
 net/net-pipe.js dur=5 type="buf" len=102400                              1.65 %            0.148484290
 net/net-pipe.js dur=5 type="buf" len=16777216                            0.05 %            0.949649889
 net/net-pipe.js dur=5 type="utf" len=102400                              0.65 %            0.463140117
 net/net-pipe.js dur=5 type="utf" len=16777216                            0.57 %            0.531757174
 net/net-s2c.js dur=5 type="asc" len=102400                               0.01 %            0.994663657
 net/net-s2c.js dur=5 type="asc" len=16777216                             0.55 %            0.690648594
 net/net-s2c.js dur=5 type="buf" len=102400                               1.06 %            0.162661878
 net/net-s2c.js dur=5 type="buf" len=16777216                             2.21 %            0.458328732
 net/net-s2c.js dur=5 type="utf" len=102400                               0.47 %            0.346382821
 net/net-s2c.js dur=5 type="utf" len=16777216                            -1.19 %            0.075676276
 net/net-wrap-js-stream-passthrough.js dur=5 type="asc" len=102400       -5.01 %            0.566507367
 net/net-wrap-js-stream-passthrough.js dur=5 type="asc" len=16777216      1.81 %            0.382296906
 net/net-wrap-js-stream-passthrough.js dur=5 type="buf" len=102400       -4.32 %            0.543143575
 net/net-wrap-js-stream-passthrough.js dur=5 type="buf" len=16777216      0.12 %            0.774690856
 net/net-wrap-js-stream-passthrough.js dur=5 type="utf" len=102400        2.33 %            0.152586683
 net/net-wrap-js-stream-passthrough.js dur=5 type="utf" len=16777216      0.50 %            0.687525683
 net/tcp-raw-c2s.js dur=5 type="asc" len=102400                           0.05 %            0.917082371
 net/tcp-raw-c2s.js dur=5 type="asc" len=16777216                         4.17 %         ** 0.005564067
 net/tcp-raw-c2s.js dur=5 type="buf" len=102400                           0.56 %          * 0.037673166
 net/tcp-raw-c2s.js dur=5 type="buf" len=16777216                         0.77 %         ** 0.006890503
 net/tcp-raw-c2s.js dur=5 type="utf" len=102400                          -0.50 %            0.397862701
 net/tcp-raw-c2s.js dur=5 type="utf" len=16777216                         1.00 %            0.300638263
 net/tcp-raw-pipe.js dur=5 type="asc" len=102400                          0.82 %            0.722353484
 net/tcp-raw-pipe.js dur=5 type="asc" len=16777216                       15.00 %            0.070918075
 net/tcp-raw-pipe.js dur=5 type="buf" len=102400                         -1.03 %            0.819639125
 net/tcp-raw-pipe.js dur=5 type="buf" len=16777216                       18.35 %            0.329069149
 net/tcp-raw-pipe.js dur=5 type="utf" len=102400                         -0.27 %            0.984576346
 net/tcp-raw-pipe.js dur=5 type="utf" len=16777216                        2.78 %            0.362840470
 net/tcp-raw-s2c.js dur=5 type="asc" len=102400                          -0.15 %            0.820491736
 net/tcp-raw-s2c.js dur=5 type="asc" len=16777216                        -0.42 %            0.813160796
 net/tcp-raw-s2c.js dur=5 type="buf" len=102400                           0.26 %            0.615102013
 net/tcp-raw-s2c.js dur=5 type="buf" len=16777216                        -2.16 %            0.464289164
 net/tcp-raw-s2c.js dur=5 type="utf" len=102400                          -0.33 %            0.383964275
 net/tcp-raw-s2c.js dur=5 type="utf" len=16777216                         1.08 %            0.224603980

As a heads up, after this I’d like to also refactor the writable side a bit more by removing WriteWrap and ShutdownWrap as explicit classes; I have some WIP work but haven’t gotten that to pass all TLS tests yet…

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

src

Merge request reports