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

Store optimizations: the JSON parts #2541

Merged

Conversation

Projects
None yet
3 participants
@rustyrussell
Copy link
Contributor

commented Apr 8, 2019

You've reviewed these before, except for the two fixup! commits from your suggestions and the new "
lightning-cli: do pretty-printing" at 72c4e42.

Otherwise, it's just a rebase, with benchmark numbers added to the commit msgs.

rustyrussell added some commits Apr 8, 2019

gossipd: use iterator helpers for iterating node channels.
Makes the next step easier.

MCP results from 5 runs, min-max(mean +/- stddev):
	store_load_msec:45791-46917(46330.4+/-3.6e+02)
	vsz_kb:2641316
	store_rewrite_sec:47.040000-48.720000(47.684+/-0.57)
	listnodes_sec:1.140000-1.340000(1.2+/-0.072)
	listchannels_sec:50.970000-54.250000(52.698+/-1.3)
	routing_sec:29.950000-31.010000(30.332+/-0.37)
	peer_write_all_sec:51.570000-52.970000(52.1+/-0.54)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossipd: use simple inline array for nodes with few channels.
Allocating a htable is overkill for most nodes; we can fit 11 pointers
in the same space (10, since we use 1 to indicate we're using an array).

MCP results from 5 runs, min-max(mean +/- stddev):
	store_load_msec:45947-47016(46683.4+/-4e+02)
	vsz_kb:2639240
	store_rewrite_sec:46.950000-49.830000(48.048+/-0.95)
	listnodes_sec:1.090000-1.350000(1.196+/-0.095)
	listchannels_sec:48.960000-57.640000(53.358+/-2.8)
	routing_sec:29.990000-33.880000(31.088+/-1.4)
	peer_write_all_sec:49.360000-53.210000(51.338+/-1.4)

MCP notable changes from previous patch (>1 stddev):
-	vsz_kb:2641316
+	vsz_kb:2639240

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
ccan: update to get smaller htable implementation.
MCP results from 5 runs, min-max(mean +/- stddev):
	store_rewrite_sec:46.750000-48.280000(47.512+/-0.51)
	listnodes_sec:1.100000-1.400000(1.192+/-0.11)
	listchannels_sec:49.530000-56.410000(53.914+/-2.3)
	routing_sec:29.500000-32.970000(30.392+/-1.3)
	peer_write_all_sec:50.760000-52.140000(51.348+/-0.59)

MCP notable changes from previous patch (>1 stddev):
-	vsz_kb:2639240
+	vsz_kb:2638880
pubkey: rename PUBKEY_DER_LEN to PUBKEY_CMPR_LEN.
Pubkeys are not not actually DER encoding, but Pieter Wuille corrected
me: it's SEC 1 documented encoding.

Results from 5 runs, min-max(mean +/- stddev):
	store_load_msec,vsz_kb,store_rewrite_sec,listnodes_sec,listchannels_sec,routing_sec,peer_write_all_sec
	38922-39297(39180.6+/-1.3e+02),2880728,41.040000-41.160000(41.106+/-0.05),2.270000-2.530000(2.338+/-0.097),44.570000-53.980000(49.696+/-3),32.840000-33.080000(32.95+/-0.095),43.060000-44.950000(43.696+/-0.72)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
common/node_id: new type.
Node ids are pubkeys, but we only use them as pubkeys for routing and checking
gossip messages.  So we're packing and unpacking them constantly, and wasting
some space and time.

This introduces a new type, explicitly the SEC1 compressed encoding
(33 bytes).  We ensure its validity when we load from the db, or get it
from JSON.  We still use 'struct pubkey' for peer messages, which checks
validity.

Results from 5 runs, min-max(mean +/- stddev):
	store_load_msec,vsz_kb,store_rewrite_sec,listnodes_sec,listchannels_sec,routing_sec,peer_write_all_sec
	39475-39572(39518+/-36),2880732,41.150000-41.390000(41.298+/-0.085),2.260000-2.550000(2.336+/-0.11),44.390000-65.150000(58.648+/-7.5),32.740000-33.020000(32.89+/-0.093),44.130000-45.090000(44.566+/-0.32)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

@rustyrussell rustyrussell added this to the 0.7.1 milestone Apr 8, 2019

@rustyrussell rustyrussell requested a review from niftynei Apr 8, 2019

@rustyrussell rustyrussell requested a review from cdecker as a code owner Apr 8, 2019

rustyrussell added some commits Apr 8, 2019

Use node_id everywhere for nodes.
I tried to just do gossipd, but it was uncontainable, so this ended up being
a complete sweep.

We didn't get much space saving in gossipd, even though we should save
24 bytes per node.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
lightning-cli: do pretty-printing.
Plugins don't do it right anyway, and we're about to remove it from
lightningd.  Produces same format as json_pp.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
json_add_hex: wire in at a lower level.
This is one of the more significant fields we print, but there's no
need to allocate a temp buffer or escape the resulting string.

MCP results from 5 runs, min-max(mean +/- stddev):
	store_load_msec:34048-36002(35070.4+/-8.5e+02)
	vsz_kb:2637488
	store_rewrite_sec:35.110000-38.120000(36.604+/-1.2)
	listnodes_sec:0.830000-1.020000(0.95+/-0.065)
	listchannels_sec:48.560000-55.680000(52.642+/-2.7)
	routing_sec:29.800000-33.170000(30.536+/-1.3)
	peer_write_all_sec:49.260000-52.490000(50.316+/-1.1)

MCP notable changes from previous patch (>1 stddev):
	-listchannels_sec:55.390000-58.110000(56.998+/-0.99)
	+listchannels_sec:48.560000-55.680000(52.642+/-2.7)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
wire: use struct node_id for node ids.
Don't turn them to/from pubkeys implicitly.  This means nodeids in the store
don't get converted, but bitcoin keys still do.

MCP results from 5 runs, min-max(mean +/- stddev):
	store_load_msec:33934-35251(34531.4+/-5e+02)
	vsz_kb:2637488
	store_rewrite_sec:34.720000-35.130000(34.94+/-0.14)
	listnodes_sec:1.020000-1.290000(1.146+/-0.086)
	listchannels_sec:51.110000-58.240000(54.826+/-2.5)
	routing_sec:30.000000-33.320000(30.726+/-1.3)
	peer_write_all_sec:50.370000-52.970000(51.646+/-1.1)

MCP notable changes from previous patch (>1 stddev):
	-store_load_msec:46184-47474(46673.4+/-4.5e+02)
	+store_load_msec:33934-35251(34531.4+/-5e+02)
	-vsz_kb:2638880
	+vsz_kb:2637488
	-store_rewrite_sec:46.750000-48.280000(47.512+/-0.51)
	+store_rewrite_sec:34.720000-35.130000(34.94+/-0.14)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
lightningd: speed low-level json formatting.
Make json_start_member allocate extra space, which caller can directly
print into, and also make caller call js_written_some() itself.

MCP results from 5 runs, min-max(mean +/- stddev):
	store_load_msec:35071-36817(35617.2+/-7e+02)
	vsz_kb:2637488
	store_rewrite_sec:35.790000-37.500000(36.6375+/-0.63)
	listnodes_sec:0.690000-0.780000(0.72+/-0.035)
	listchannels_sec:34.600000-36.340000(35.36+/-0.77)
	routing_sec:30.310000-30.730000(30.445+/-0.17)
	peer_write_all_sec:50.830000-52.750000(51.82+/-0.89)

MCP notable changes from previous patch (>1 stddev):
	-listnodes_sec:0.720000-0.950000(0.86+/-0.077)
	+listnodes_sec:0.690000-0.780000(0.72+/-0.035)
	-listchannels_sec:40.300000-41.080000(40.668+/-0.29)
	+listchannels_sec:34.600000-36.340000(35.36+/-0.77)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossipd: make more compact getchannels entries.
We can save significant space by combining both sides: so much that we
can reduce the WIRE_LEN_LIMIT to something sane again.

MCP results from 5 runs, min-max(mean +/- stddev):
	store_load_msec:34467-36764(35517.8+/-7.7e+02)
	vsz_kb:2637488
	store_rewrite_sec:35.310000-36.580000(35.816+/-0.44)
	listnodes_sec:1.140000-2.780000(1.596+/-0.6)
	listchannels_sec:55.390000-58.110000(56.998+/-0.99)
	routing_sec:30.330000-30.920000(30.642+/-0.19)
	peer_write_all_sec:50.640000-53.360000(51.822+/-0.91)

MCP notable changes from previous patch (>1 stddev):
	-store_rewrite_sec:34.720000-35.130000(34.94+/-0.14)
	+store_rewrite_sec:35.310000-36.580000(35.816+/-0.44)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
lightningd: don't bother pretty-printing JSON.
This doesn't result in a speedup for our benchmark, since we use the
cli tool which does the formatting.

MCP results from 5 runs, min-max(mean +/- stddev):
	store_load_msec:33422-36830(35196.2+/-1.2e+03)
	vsz_kb:2637488
	store_rewrite_sec:36.030000-37.630000(36.794+/-0.52)
	listnodes_sec:0.720000-0.950000(0.86+/-0.077)
	listchannels_sec:40.300000-41.080000(40.668+/-0.29)
	routing_sec:30.440000-31.030000(30.69+/-0.2)
	peer_write_all_sec:50.060000-52.800000(51.416+/-0.91)

MCP notable changes from 2 patches ago (>1 stddev):
	-listchannels_sec:48.560000-55.680000(52.642+/-2.7)
	+listchannels_sec:40.300000-41.080000(40.668+/-0.29)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
lightingd: optimize json_add_short_channel_id a little.
MCP results from 5 runs, min-max(mean +/- stddev):
	store_load_msec:32825-36365(34615.6+/-1.1e+03)
	vsz_kb:2637488
	store_rewrite_sec:35.150000-36.200000(35.59+/-0.4)
	listnodes_sec:0.590000-0.710000(0.682+/-0.046)
	listchannels_sec:28.650000-31.080000(29.966+/-0.81)
	routing_sec:29.980000-33.100000(31.284+/-1.5)
	peer_write_all_sec:49.020000-52.890000(50.376+/-1.5)

MCP notable changes from previous patch (>1 stddev):
	-store_rewrite_sec:35.790000-37.500000(36.6375+/-0.63)
	+store_rewrite_sec:35.150000-36.200000(35.59+/-0.4)
	-listchannels_sec:34.600000-36.340000(35.36+/-0.77)
	+listchannels_sec:28.650000-31.080000(29.966+/-0.81)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

... Trivial rebase to fix include order...

@rustyrussell rustyrussell force-pushed the rustyrussell:guilt/store-load-optimize branch from af3bc90 to d3d741f Apr 8, 2019

@SimonVrouwe

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

Have you thought about stop logging IO when accessing the IO log?

EDIT: Sorry, I am in the wrong PR

@niftynei

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

ACK d3d741f

@niftynei

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

should we add the new -R flag to the Changelog?

@niftynei niftynei merged commit 4811024 into ElementsProject:master Apr 9, 2019

2 checks passed

ackbot PR ack'd by niftynei
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rustyrussell rustyrussell added the MCP label May 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.