Rusty Russell [Mon, 24 Jun 2024 02:45:02 +0000 (12:15 +0930)]
io: add benchmark for large poll.
This simulates a performance issue reported in `connectd` for
lightningd with 900 fds: it's consuming 70% of a CPU, and strace shows
it getting a single fd (or two) from poll, doing a write (gossip store
streaming), then going back into poll.
I tested using 1000 x netcat from a machine on the same LAN:
```
remote$ for i in `seq 1000`; do sleep 0.1; nc 10.88.9.17 8888 </dev/null > /dev/null & done
```
Rusty Russell [Sun, 3 Mar 2024 22:33:11 +0000 (09:03 +1030)]
rune: make error message better when integer parameter is missing.
We say "is not an integer field" when a field is missing entirely,
which is technically true but confusing. Say "not present" like
every other condition does.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
David Gibson [Sat, 1 Jun 2024 09:03:27 +0000 (19:03 +1000)]
bitmap: Avoid shadowing type name with parameter name
'bitmap' is a typedef name in this module, and we also use it as a
parameter name in a bunch of places. Although that's possible, because
types and variables live in different namespaces, it's probably not that
wise. It also appears to trip warnings with at least some options and
compiler versions.
Reported-by: Ashok Raj <ashok.raj@intel.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Rusty Russell [Tue, 1 Aug 2023 01:43:53 +0000 (11:13 +0930)]
base64: fix for unsigned chars (e.g. ARM).
```
ccan/ccan/base64/base64.c:34:10: error: result of comparison of constant 255 with expression of type 'int8_t' (aka 'signed char') is always false [-Werror,-Wtautological-constant-out-of-range-compare]
if (ret == (char)0xff) {
~~~ ^ ~~~~~~~~~~
ccan/ccan/base64/base64.c:44:57: error: result of comparison of constant 255 with expression of type 'const signed char' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
return (maps->decode_map[(const unsigned char)b64char] != (char)0xff);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~
```
Reported-by: Christian Decker Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell [Mon, 9 Jan 2023 02:06:22 +0000 (12:36 +1030)]
tcon: fix warning when it's used with NULL on some gcc versions.
Seems to be happening with gcc 12.2.0-3ubuntu1:
```
lightningd/jsonrpc.c: In function ‘destroy_json_command’:
lightningd/jsonrpc.c:1180:63: error: the comparison will always evaluate as ‘false’ for the address of ‘canary’ will never be NULL [-Werror=address]
lightningd/jsonrpc.c:108:53: note: ‘canary’ declared here
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell [Mon, 9 Jan 2023 02:16:22 +0000 (12:46 +1030)]
idtree: make sanitizer happier about shifts.
```
/home/rusty/devel/cvs/ccan/ccan/idtree/test/run-wrap.c:/home/rusty/devel/cvs/ccan/ccan/idtree/idtree.c:76:21: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell [Mon, 9 Jan 2023 02:15:22 +0000 (12:45 +1030)]
version: fix version() function.
```
/home/rusty/devel/cvs/ccan/ccan/version/test/run.c:/home/rusty/devel/cvs/ccan/ccan/version/version.h:58:35: runtime error: left shift of 65535 by 16 places cannot be represented in type 'int'
```
Rusty Russell [Mon, 9 Jan 2023 02:09:22 +0000 (12:39 +1030)]
tcon: fix overzealous gcc 12.2.0 warning.
At least with -fsanitize=address -fno-sanitize-recover=address -fsanitize=undefined -fno-sanitize-recover=undefined:
```
/home/rusty/devel/cvs/ccan/ccan/tcon/test/compile_fail-container1.c:Compile gave warnings without -DFAIL:
In file included from /home/rusty/devel/cvs/ccan/ccan/tcon/test/compile_fail-container1.c:3:
/home/rusty/devel/cvs/ccan/ccan/tcon/tcon.h: In function ‘main’:
/home/rusty/devel/cvs/ccan/ccan/tcon/tcon.h:332:25: warning: ‘innerp’ may be used uninitialized [-Wmaybe-uninitialized]
332 | canary, tcon_container_of_((member_ptr), \
| ^~~~~~~~~~~~~~~~~~
/home/rusty/devel/cvs/ccan/ccan/tcon/tcon.h:335:21: note: by argument 1 of type ‘const void *’ to ‘tcon_container_of_’ declared here
335 | static inline void *tcon_container_of_(const void *member_ptr, size_t offset)
| ^~~~~~~~~~~~~~~~~~
/home/rusty/devel/cvs/ccan/ccan/tcon/test/compile_fail-container1.c:28:22: note: ‘ovar’ declared here
28 | struct outer ovar;
| ^~~~
Rusty Russell [Mon, 4 Jul 2022 00:11:57 +0000 (09:41 +0930)]
ccan/rune: compile without warnings on -O3.
```
ccan/ccan/rune/rune.c: In function ‘rune_alt_single_int’:
ccan/ccan/rune/rune.c:257:5: error: ‘runeval_int’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
257 | if (cond)
| ^
ccan/ccan/rune/rune.c:305:6: note: ‘runeval_int’ was declared here
305 | s64 runeval_int;
| ^~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:919: ccan-rune-rune.o] Error 1
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell [Fri, 17 Jun 2022 04:34:14 +0000 (14:04 +0930)]
htable: opportunistically avoid delete marker.
Fairly cheap test, we can see a drop in initial re-inserting, probably
because it has just been cleaned at this point with the old code, but
generally we do better with churn.
I also tried a variant which only checked if next bucket was on same
cacheline, but no significant difference.
Rusty Russell [Mon, 13 Jun 2022 12:18:19 +0000 (21:48 +0930)]
htable: fix vanishing entries properly.
A hash table entry consists of the pointer, but we mask out bits which
are common to all pointers, and replace them with hash bits.
The entry values 0 and 1 are special, meaning "empty" and "deleted"
respectively.
However, if a hash has mainly zero bits, and an pointer's non-shared
bits are all zero, we can create an entry which looks empty or
deleted.
The solution in this case is to share another bit (there must be some
bit which is not zero, since we don't allow 0 and 1 as pointers in the
hash table).
It's unusual, so I put this in a cold function: it shares code with
the existing "oh, we need to reduce the common bits, since this new
pointer doesn't look like the previous ones" case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Except for:
- Leaves the test behind though (which fail!
- Leaves the reversion of 0b93bd102aad6b61f1e569fb12aabc6352a1d7cd, since
we're about to fix it, and properly!
Rusty Russell [Thu, 9 Jun 2022 03:12:31 +0000 (12:42 +0930)]
htable: handle v. unlikely case where entries look deleted/empty.
If the hash function doesn't set any bits we use, and the common
bits are all zero the slot will look empty (or, just the lower bit is
set: the slow looks deleted).
However, each bucket is distinct since there are no duplicates, so
at worse there can be two "looks invalid but actually is valid"
buckets. Keep them at the end.
Lookup suffers in raw tools/speed though:
-Lookup after half-change (match): 53-61(54.8+/-2.3) ns
+Lookup after half-change (match): 61-113(83.3+/-17) ns
-Lookup after churn & spread (match): 54-90(59.9+/-11) ns
+Lookup after churn & spread (match): 70-108(85.2+/-14) ns
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell [Fri, 17 Jul 2020 01:56:51 +0000 (11:26 +0930)]
tools/configurator: fix compile error with -O2
In file included from /usr/include/string.h:495,
from configuratortest.c:2:
In function ‘strncpy’,
inlined from ‘main’ at configuratortest.c:6:2:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: warning: ‘__builtin_strncpy’ specified bound 8 equals destination size [-Wstringop-truncation]
106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell [Mon, 17 Feb 2020 00:12:46 +0000 (10:42 +1030)]
tal: don't defer-after-free if a destructor deletes itself.
==10868== at 0x109A96: notify (tal.c:220)
==10868== by 0x109F7E: del_tree (tal.c:397)
==10868== by 0x10A31A: tal_free (tal.c:481)
==10868== by 0x10BE73: main (run-destructor.c:75)
==10868== Address 0x4a60bd8 is 8 bytes inside a block of size 32 free'd
==10868== at 0x483BA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==10868== by 0x109D4F: del_notifier_property (tal.c:340)
==10868== by 0x10A610: tal_del_notifier_ (tal.c:564)
==10868== by 0x10A687: tal_del_destructor_ (tal.c:576)
==10868== by 0x10B653: remove_own_destructor (run-destructor.c:35)
==10868== by 0x109A67: notify (tal.c:235)
==10868== by 0x109F7E: del_tree (tal.c:397)
==10868== by 0x10A31A: tal_free (tal.c:481)
==10868== by 0x10BE73: main (run-destructor.c:75)
==10868== Block was alloc'd at
==10868== at 0x483A7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==10868== by 0x109AD5: allocate (tal.c:245)
==10868== by 0x109C3E: add_notifier_property (tal.c:300)
==10868== by 0x10A467: tal_add_destructor_ (tal.c:516)
==10868== by 0x10BDFE: main (run-destructor.c:74)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell [Tue, 11 Feb 2020 04:09:43 +0000 (14:39 +1030)]
opt: fix opt_unregister.
Instead of memmoving N structs, we were memmoving N bytes.
But why did the test pass then? It was doing memmove(..., 1)
instead of memmove(..., sizeof(struct opt_table)!
Because the two structures were really similar; the main difference
was the first entry, which points to the name. But they were allocated
consecutively, and Intel being little-endian, the only difference was
the first byte! Thus memmove(1) was enough to make it "work".
Change two options in the test to be sufficiently different, and
the bug shows up.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Niklas Claesson [Tue, 28 Jan 2020 23:50:15 +0000 (00:50 +0100)]
configurator: Enable running configurator in wrapper
Since the probing binaries compiled by the configurator needs to run on
the host machine we provide a variable CONFIGURATOR_WRAPPER that can be
set to anything that you want to wrap the calls with.
Kirill Smelkov [Mon, 21 Oct 2019 15:09:20 +0000 (15:09 +0000)]
bitmap: Allow bitmap type to be forward declared
Currently bitmap type is defined via untagged struct which makes it
impossible to forward declare it. Forward-declaring is useful since all
bitmap functions only use bitmap* and in public user-visible
headers/datastructures it is enough to indicate that a data field with
bitmap pointer is there, whereas bitmap.h can be included only in
implementation.
Beside that some headers are included by both C and C++ parts of a
project, and when ccan/bitmap.h is processed by C++ compiler it gives:
./3rdparty/ccan/ccan/bitmap/bitmap.h: In function ‘bitmap* bitmap_alloc(long unsigned int)’:
./3rdparty/ccan/ccan/bitmap/bitmap.h:201:15: error: invalid conversion from ‘void*’ to ‘bitmap*’ [-fpermissive]
return malloc(bitmap_sizeof(nbits));
~~~~~~^~~~~~~~~~~~~~~~~~~~~~
./3rdparty/ccan/ccan/bitmap/bitmap.h: In function ‘bitmap* bitmap_realloc0(bitmap*, long unsigned int, long unsigned int)’:
./3rdparty/ccan/ccan/bitmap/bitmap.h:227:18: error: invalid conversion from ‘void*’ to ‘bitmap*’ [-fpermissive]
bitmap = realloc(bitmap, bitmap_sizeof(nbits));
~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./3rdparty/ccan/ccan/bitmap/bitmap.h: In function ‘bitmap* bitmap_realloc1(bitmap*, long unsigned int, long unsigned int)’:
./3rdparty/ccan/ccan/bitmap/bitmap.h:238:18: error: invalid conversion from ‘void*’ to ‘bitmap*’ [-fpermissive]
bitmap = realloc(bitmap, bitmap_sizeof(nbits));
-> Give to users ability not to force-include ccan/bitmap.h by
forward-declaring bitmaps like this:
projects / ccan / log