Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(concatenated string) takes 23% of beacon node's heap #3446

Closed
dapplion opened this issue Nov 19, 2021 · 10 comments
Closed

(concatenated string) takes 23% of beacon node's heap #3446

dapplion opened this issue Nov 19, 2021 · 10 comments
Labels
prio-high Resolve issues as soon as possible. scope-memory Issues to reduce and improve memory usage.

Comments

@dapplion
Copy link
Contributor

Describe the bug

In a synced healthy node in mainnet, according to a recent heap snapshot (concatenated string) take

  • 23% of the heap
  • 366MB
  • 11M instances

Screenshot from 2021-11-19 22-29-21

Expected behavior

??? Strings should take so much memory?

@dapplion dapplion added the scope-memory Issues to reduce and improve memory usage. label Nov 19, 2021
@dapplion
Copy link
Contributor Author

dapplion commented Nov 21, 2021

I've manually opened 2021-11-21T10:09:09.713Z.heapsnapshot with Sublime Text (took 18GB RAM lol) and inspected the "strings" array, which lists all individual strings. Reference to the heapsnapshot format https://github.com/v8/v8/blob/17a99fec258bcc07ea9fc5e4fabcce259751db03/include/v8-profiler.h#L586

Search for "strings":["<dummy>",, in the above referenced heapsnapshot it starts at the line number 216_203_711

Notable strange things found:

Base64 text (extreme amounts)

m6Iw+9ey1UiRGGI0lCv5DHdbf0YE
mnnGLYdBbZ5B/DfFotXGtfJQEiPQ
mvyxSol88xUg8HPjigsyXFZQPU+s

Seem to be part of PeerScore.deliveryRecords and MessageDeliveries.records

Short hex values (extreme amounts)

0x9983659130
0x8876951e38
0x8ecb05a455

From #3446 (comment)

Code snippets

/* -*- Mode: js; js-indent-level: 2; -*- */\n/*\n * Copyright 2011 Mozilla Foundation and contributors\n * Licensed under the New BSD license. See LICENSE or:\n * http://opensource.org/licenses/BSD-3-Clause\n */\n\nexports.GREATEST_LOWER_BOUND = 1;\nexports.LEAST_UPPER_BOUND = 2;\n\n/**\n * Recursive implementation of binary search.\n *\n * @param aLow Indices here and lower do not contain the needle.\n * @param aHigh Indices here and higher do not contain the needle.\n * @param aNeedle The element being searched for.\n * @param aHaystack The non-empty array being searched.\n * @param aCompare Function which takes two elements and returns -1, 0, or 1.\n * @param aBias Either 'binarySearch.GREATEST_LOWER_BOUND' or\n *     'binarySearch.LEAST_UPPER_BOUND'. Specifies whether to return the\n *     closest element that is smaller than or greater than the one we are\n *     searching for, respectively, if the exact element cannot be found.\n */\nfunction recursiveSearch(aLow, aHigh, aNeedle, aHaystack, aCompare, aBias) {\n  // This functi
/* -*- Mode: js; js-indent-level: 2; -*- */\n/*\n * Copyright 2011 Mozilla Foundation and contributors\n * Licensed under the New BSD license. See LICENSE or:\n * http://opensource.org/licenses/BSD-3-Clause\n */\n\n// It turns out that some (most?) JavaScript engines don't self-host\n// `Array.prototype.sort`. This makes sense because C++ will likely remain\n// faster than JS when doing raw CPU-intensive sorting. However, when using a\n// custom comparator function, calling back and forth between the VM's C++ and\n// JIT'd JS is rather slow *and* loses JIT type information, resulting in\n// worse generated code for the comparator function than would be optimal. In\n// fact, when sorting with a comparator, these costs outweigh the benefits of\n// sorting in C++. By using our own JS-implemented Quick Sort (below), we get\n// a ~3500ms mean speed-up in `bench/bench.html`.\n\n/**\n * Swap the elements indexed by `x` and `y` in the array `ary`.\n *\n * @param {Array} ary\n *        The array.\n * @param {Number} x\n *        The index o
/* -*- Mode: js; js-indent-level: 2; -*- */\n/*\n * Copyright 2011 Mozilla Foundation and contributors\n * Licensed under the New BSD license. See LICENSE or:\n * http://opensource.org/licenses/BSD-3-Clause\n */\n\nvar SourceMapGenerator = require('./source-map-generator').SourceMapGenerator;\nvar util = require('./util');\n\n// Matches a Windows-style `\\r\\n` newline or a `\\n` newline used by all other\n// operating systems these days (capturing the result).\nvar REGEX_NEWLINE = /(\\r?\\n)/;\n\n// Newline character code for charCodeAt() comparisons\nvar NEWLINE_CODE = 10;\n\n// Private symbol for identifying `SourceNode`s when multiple versions of\n// the source-map library are loaded. This MUST NOT CHANGE across\n// versions!\nvar isSourceNode = \"$$$isSourceNode$$$\";\n\n/**\n * SourceNodes provide a way to abstract over interpolating/concatenating\n * snippets of generated JavaScript source code while maintaining the line and\n * column information associated with the original source code.\n *\n * @param aLine The original line number.\n

domains?

film.museum
fineart.museum
finearts.museum
matsuura.nagasaki.jp
nagasaki.nagasaki.jp
obama.nagasaki.jp

From https://github.com/lupomontero/psl/blob/master/data/rules.json

RSA Certificates LOL

-----BEGIN CERTIFICATE-----\nMIIEQzCCAyugAwIBAgIDCYP0MA0GCSqGSIb3DQEBCwUAMFAxCzAJBgNVBAYTAkRFMRUwEwYD\nVQQKDAxELVRydXN0IEdtYkgxKjAoBgNVBAMMIUQtVFJVU1QgUm9vdCBDbGFzcyAzIENBIDIg\nRVYgMjAwOTAeFw0wOTExMDUwODUwNDZaFw0yOTExMDUwODUwNDZaMFAxCzAJBgNVBAYTAkRF\nMRUwEwYDVQQKDAxELVRydXN0IEdtYkgxKjAoBgNVBAMMIUQtVFJVU1QgUm9vdCBDbGFzcyAz\nIENBIDIgRVYgMjAwOTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAJnxhDRwui+3\nMKCOvXwEz75ivJn9gpfSegpnljgJ9hBOlSJzmY3aFS3nBfwZcyK3jpgAvDw9rKFs+9Z5JUut\n8Mxk2og+KbgPCdM03TP1YtHhzRnp7hhPTFiu4h7WDFsVWtg6uMQYZB7jM7K1iXdODL/ZlGsT\nl28So/6ZqQTMFexgaDbtCHu39b+T7WYxg4zGcTSHThfqr4uRjRxWQa4iN1438h3Z0S0NL2lR\np75mpoo6Kr3HGrHhFPC+Oh25z1uxav60sUYgovseO3Dvk5h9jHOW8sXvhXCtKSb8HgQ+HKDY\nD8tSg2J87otTlZCpV6LqYQXY+U3EJ/pure3511H3a6UCAwEAAaOCASQwggEgMA8GA1UdEwEB\n/wQFMAMBAf8wHQYDVR0OBBYEFNOUikxiEyoZLsyvcop9NteaHNxnMA4GA1UdDwEB/wQEAwIB\nBjCB3QYDVR0fBIHVMIHSMIGHoIGEoIGBhn9sZGFwOi8vZGlyZWN0b3J5LmQtdHJ1c3QubmV0\nL0NOPUQtVFJVU1QlMjBSb290JTIwQ2xhc3MlMjAzJTIwQ0ElMjAyJTIwRVYlMjAyMDA5\nRC1UcnVzdCUyMEdtYkgsQz1ERT9jZXJ0aWZpY2F0ZXJldm9
-----BEGIN CERTIFICATE-----\nMIIFaTCCA1GgAwIBAgIJAJK4iNuwisFjMA0GCSqGSIb3DQEBCwUAMFIxCzAJBgNVBAYTAlNL\nMRMwEQYDVQQHEwpCcmF0aXNsYXZhMRMwEQYDVQQKEwpEaXNpZyBhLnMuMRkwFwYDVQQDExBD\nQSBEaXNpZyBSb290IFIyMB4XDTEyMDcxOTA5MTUzMFoXDTQyMDcxOTA5MTUzMFowUjELMAkG\nA1UEBhMCU0sxEzARBgNVBAcTCkJyYXRpc2xhdmExEzARBgNVBAoTCkRpc2lnIGEucy4xGTAX\nBgNVBAMTEENBIERpc2lnIFJvb3QgUjIwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoIC\nAQCio8QACdaFXS1tFPbCw3OeNcJxVX6B+6tGUODBfEl45qt5WDza/3wcn9iXAng+a0EE6UG9\nvgMsRfYvZNSrXaNHPWSb6WiaxswbP7q+sos0Ai6YVRn8jG+qX9pMzk0DIaPY0jSTVpbLTAwA\nFjxfGs3Ix2ymrdMxp7zo5eFm1tL7A7RBZckQrg4FY8aAamkw/dLukO8NJ9+flXP04SXabBbe\nQTg06ov80egEFGEtQX6sx3dOy1FU+16SGBsEWmjGycT6txOgmLcRK7fWV8x8nhfRyyX+hk4k\nLlYMeE2eARKmK6cBZW58Yh2EhN/qwGu1pSqVg8NTEQxzHQuyRpDRQjrOQG6Vrf/GlK1ul4SO\nfW+eioANSW1z4nuSHsPzwfPrLgVv2RvPN3YEyLRa5Beny912H9AZdugsBbPWnDTYltxhh5EF\n5EQIM8HauQhl1K6yNg3ruji6DOWbnuuNZt2Zz9aJQfYEkoopKW1rOhzndX0CcQ7zwOe9yxnd\nnWCywmZgtrEE7snmhrmaZkCo5xHtgUUDi/ZnWejBBhG93c+AAk9lQHhcR1DIm+YfgXvkRKhb\nhZri3lrVx/k6RGZL5DJUfORsnLMOPReisjQS1n6yqEm
-----BEGIN CERTIFICATE-----\nMIIH0zCCBbugAwIBAgIIXsO3pkN/pOAwDQYJKoZIhvcNAQEFBQAwQjESMBAGA1UEAwwJQUND\nVlJBSVoxMRAwDgYDVQQLDAdQS0lBQ0NWMQ0wCwYDVQQKDARBQ0NWMQswCQYDVQQGEwJFUzAe\nFw0xMTA1MDUwOTM3MzdaFw0zMDEyMzEwOTM3MzdaMEIxEjAQBgNVBAMMCUFDQ1ZSQUlaMTEQ\nMA4GA1UECwwHUEtJQUNDVjENMAsGA1UECgwEQUNDVjELMAkGA1UEBhMCRVMwggIiMA0GCSqG\nSIb3DQEBAQUAA4ICDwAwggIKAoICAQCbqau/YUqXry+XZpp0X9DZlv3P4uRm7x8fRzPCRKPf\nmt4ftVTdFXxpNRFvu8gMjmoYHtiP2Ra8EEg2XPBjs5BaXCQ316PWywlxufEBcoSwfdtNgM38\n02/J+Nq2DoLSRYWoG2ioPej0RGy9ocLLA76MPhMAhN9KSMDjIgro6TenGEyxCQ0jVn8ETdkX\nhBilyNpAlHPrzg5XPAOBOp0KoVdDaaxXbXmQeOW1tDvYvEyNKKGno6e6Ak4l0Squ7a4DIrhr\nIA8wKFSVf+DuzgpmndFALW4ir50awQUZ0m/A8p/4e7MCQvtQqR0tkw8jq8bBD5L/0KIV9VMJ\ncRz/RROE5iZe+OCIHAr8Fraocwa48GOEAqDGWuzndN9wrqODJerWx5eHk6fGioozl2A3ED6X\nPm4pFdahD9GILBKfb6qkxkLrQaLjlUPTAYVtjrs78yM2x/474KElB0iryYl0/wiPgL/AlmXz\n7uxLaL2diMMxs0Dx6M/2OLuc5NF/1OVYm3z61PMOm3WR5LpSLhl+0fXNWhn8ugb2+1KoS5kE\n3fj5tItQo05iifCHJPqDQsGH+tUtKSpacXpkatcnYGMN285J9Y0fkIkyF/hzQ7jSWpOGYdbh\ndQrqeWZ2iE9x6wQl1gpaepPluUsXQA+xtrn13k/c4L

@dapplion
Copy link
Contributor Author

In the (strings) section in Chrome devtools identical strings appear identified by different node ID
Screenshot from 2021-11-21 23-28-04
Screenshot from 2021-11-21 23-27-38

Screenshot from 2021-11-21 23-32-12

Are they consuming extra memory or is a rendering error?

@dapplion
Copy link
Contributor Author

dapplion commented Nov 21, 2021

The short hex values are individual parts of large hex values converted using toHexString()

Screenshot from 2021-11-21 23-35-43

@dapplion
Copy link
Contributor Author

dapplion commented Nov 21, 2021

node_modules/bip39/src/wordlists is polluting many strings into the heap. Remove from Lodestar's bundle.

Also review node_modules/psl/data/rules.json

@philknows philknows added the prio-high Resolve issues as soon as possible. label Nov 29, 2021
@dapplion
Copy link
Contributor Author

dapplion commented Jan 3, 2022

I've run some memory benchmarks

Bytes32 toHexString()                      - 901.1 bytes / instance
Bytes32 Buffer.toString(hex)               - 85.4 bytes / instance
Bytes32 Buffer.toString(hex) Buffer.from() - 84.8 bytes / instance
Bytes32 Buffer.toString(hex) + 0x          - 121.7 bytes / instance

and performance benchmarks

misc / bytes32 to hex
✓ bytes32 toHexString                                                  1184834 ops/s    844.0000 ns/op        -     346342 runs  0.404 s
✓ bytes32 Buffer.toString(hex)                                         1669449 ops/s    599.0000 ns/op        -     775931 runs  0.707 s
✓ bytes32 Buffer.toString(hex) + 0x                                    1683502 ops/s    594.0000 ns/op        -     670067 runs  0.606 s

Representing strings as concatenated takes x10 times more memory! We must use Buffer.toString() everywhere and use some switch strategy to make it browser compatibe.

@wemeetagain
Copy link
Member

wemeetagain commented Jan 3, 2022

node_modules/bip39/src/wordlists is polluting many strings into the heap. Remove from Lodestar's bundle.

It's a dependency of @chainsafe/bls-keygen. We can remove deriveKeyFromMnemonic and get rid of the dependency there.

EDIT: Actually, it looks like we use deriveKeyFromMnemonic in a few places.

@dapplion
Copy link
Contributor Author

dapplion commented Jan 4, 2022

It's a dependency of @chainsafe/bls-keygen. We can remove deriveKeyFromMnemonic and get rid of the dependency there.

Before considering removing we should calculate the impact on memory of this specific dependency, maybe it's quite low and not an issue. Because we do need that dependency to run from mnemonic which we do in our testnet deployments

@dapplion
Copy link
Contributor Author

dapplion commented Feb 4, 2022

Latest release has 1 GB less of process heap bytes. We should take another heap snapshot to confirm the current share of concatenated strings

@dapplion
Copy link
Contributor Author

dapplion commented Feb 4, 2022

Screenshot from 2022-02-04 17-30-46

Down from 366MB to 2MB =D

@dapplion dapplion closed this as completed Feb 4, 2022
@dapplion
Copy link
Contributor Author

dapplion commented Feb 4, 2022

For reference now the strings take 112 bytes (reported) and 165 bytes (real 45535544 / 275496)

Screenshot from 2022-02-04 17-33-07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio-high Resolve issues as soon as possible. scope-memory Issues to reduce and improve memory usage.
Projects
None yet
Development

No branches or pull requests

3 participants