Skip to content

tls_wrap: Unlink TLSWrap and SecureContext objects asap.

Rodrigo Muino Tomonari requested to merge github/fork/ChALkeR/unlink-tlswrap-securecontext into master

This makes TLSWrap and SecureContext objects collectable by the incremental gc.

res = null; destroys the cyclic reference in the reading property, delete this.ssl removes the remaining reference. delete this.ssl._secureContext.context; makes SecureContext objects collectable by the incremental gc even though there might be references to this.ssl._secureContext somewhere.

The reading property will throw an error if accessed after the socket is closed. If this is unwanted then the property getter/setter should be altered. Can someone please comment on this part? Should be ok as it is.

Test results for 20000 opened-closed non keep-alive connections to localhost (100 parallel requests):

  1. b4ad5d70:
    1. Incremental GC timings: [2,3,3,5,6,6,11,11,10,10,10,11,11,9,10,10,11,10,10,11,12,10,12,10,12] = 226ms.
    2. Full GC timings: [31,29,31,30,29,30,31,22,32,29,31,30,29,29,29,30,31,29,29,19,27] = 607ms.
    3. GC runs: 25 incremental + 21 full = 46 total.
    4. Total GC time: 883ms.
    5. Average GC time: 19ms.
  2. b4ad5d70 + this pull request:
    1. Incremental GC timings: [2,3,3,5,6,6,13,11,12,11,10,11,14,10,11,12,10,13,11,12,10,11,12,10,11,13,11,11,12,10,14,10,13,10,12,11,11,10] = 388ms.
    2. Full GC timings: [16,12,13,11,12,15,13,16,13,13,14,14,13,12] = 187ms.
    3. GC runs: 38 incremental + 14 full = 52 total.
    4. Total GC time: 525ms.
    5. Average GC time: 11ms.

Merge request reports