Lower TLS memory footprint

[indutny]

See #1522

[indutny]

CI: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/603/

[ChALkeR]

Saves ~30% rss in my testcase (that consists of repeating a https request many times).
That's short-term, i haven't measured how this changes the time when full gc kicks in yet.

[ChALkeR]

Looks like not all ssl buffers are freed, see #1522 (comment)

[ChALkeR]

I guess that's in the SecureContext.
Not 100% sure, though.

[ChALkeR]

Yes, it looks like SecureContext objects are created per every request and not deleted (until a full gc run).

[indutny]

It is, and I don't think that there is something that we could do about it with enabling the keepAlive. I mean, that's the purpose of it!

[ChALkeR]

No, when the keepAlive is not enabled and the context isn't going to be reused, isn't it best to dispose of it immediately without waiting for the full gc run? As you did with the TLSWrap data here.

Results:
http://oserv.org/bugs/iojs/memory0/pr1529.t1.3000iter.without.massif.out.12103 — without this pr
http://oserv.org/bugs/iojs/memory0/pr1529.t1.3000iter.massif.out.12080 — with this pr
http://oserv.org/bugs/iojs/memory0/pr1529.t1.3000iter.contextkill.massif.out.31454 — with this pr and destroying the context with

this.on('close',function(){context.context.close();});

[indutny]

Oh, good idea...

[indutny]

@ChALkeR pushed, thank you!

[ChALkeR]

Does not work somewhy. Will check in several minutes.
This commit didn't change anything.

[indutny]

Because I made a typo, sorry!

[indutny]

Force pushed...

[shigeki]

@indutny This causes a link error with gcc-4.8.4 on my Ubuntu but works fine with gcc-4.9.2 on Ubuntu and with clang on Mac though I've not checked yet whether it comes from a gcc bug or not.

/home/ohtsu/github/shigeki/io.js/out/Release/obj.target/iojs/src/tls_wrap.o: In function `node::crypto::SSLWrap<node::TLSWrap>::~SSLWrap()':
tls_wrap.cc:(.text._ZN4node6crypto7SSLWrapINS_7TLSWrapEED0Ev[_ZN4node6crypto7SSLWrapINS_7TLSWrapEED0Ev]+0x14): undefined reference to `node::crypto::SSLWrap<node::TLSWrap>::DestroySSL()'
/home/ohtsu/github/shigeki/io.js/out/Release/obj.target/iojs/src/tls_wrap.o: In function `node::TLSWrap::~TLSWrap()':
tls_wrap.cc:(.text._ZN4node7TLSWrapD2Ev+0x1fe): undefined reference to `node::crypto::SSLWrap<node::TLSWrap>::DestroySSL()'
collect2: error: ld returned 1 exit status
iojs.target.mk:198: recipe for target '/home/ohtsu/github/shigeki/io.js/out/Release/iojs' failed
make[1]: *** [/home/ohtsu/github/shigeki/io.js/out/Release/iojs] Error 1
make[1]: Leaving directory '/home/ohtsu/github/shigeki/io.js/out'
Makefile:35: recipe for target 'iojs' failed
make: *** [iojs] Error 2
ohtsu@u1504:~/github/shigeki/io.js$ g++ --version
g++ (Ubuntu 4.8.4-1ubuntu15) 4.8.4

[ChALkeR]

Does not work.
options.keepAlive is undefined by default on the line where you compare it to false, but by default keep-alive is disabled AFAIK.

[indutny]

Oh, right! I had keepAlive: false in my test. Fixed everything!

[ChALkeR]

http://oserv.org/bugs/iojs/memory0/pr1529.t2.3000iter.massif.out.1609 — massif.out for the updated pull request.
Works for me, the total memory usage after 3000 requests (without keep-alive) went from 225 MiB down to 8 MiB (in first version of this pr — 120 MiB).
Full gc was not triggered in either of the three versions (without the patch, first version, current version).

Hope this doesn't break anything ;-).

[ChALkeR]

I will run upload the results of a longer test soon.

[rvagg]

I'd love a comparison of this to 0.12 and 0.10 if anyone can manage it, are we actually improving on what was there before or just on the TLSWrap changes, does OpenSSL 1.0.2 buy us anything here? I've heard some complaints about Node's memory footprint with tls in general in the past and it'd be great if this is attacking that.

[ChALkeR]

I'd love a comparison of this to 0.12 and 0.10 if anyone can manage it, are we actually improving on what was there before or just on the TLSWrap changes,

Afaik 0.12 behaves like iojs without this pull request, it just runs full gc earlier due to an older version of v8, so the problem wasn't so noticable. I will upload 0.12 test results with the same parameters a bit later.

[ChALkeR]

With the current version of the pr full gc was triggered after 8720 iterations, memory usage before full gc run looks like this: http://oserv.org/bugs/iojs/memory0/pr1529.t2.8720iter.massif.out.4212
Memory usage for 8720 iterations is 17 MiB.

[ChALkeR]

The two parts that grow are node::TLSWrap::Wrap() (3,3 MiB), that's the TLSWrap objects themselves and node::NodeBIO::TryAllocateForWrite() (8,5 MiB), from TLSWrap::DoWrite().
The base (non-growing part) is ~4.8 MiB.

That's about 1.4 KiB per request (was ~75 KiB per request before this pr).
node::NodeBIO::TryAllocateForWrite() is ~1KiB per request.
node::TLSWrap::Wrap() is ~0.4KiB per request.

Just in case: is there something that could be done with node::NodeBIO::TryAllocateForWrite()?