andrew_mo_ has quit [Read error: Connection reset by peer]
<luke-jr>
I feel like virtual size stuff needs simplification somehow, but it's annoying due to the project modularity boundaries and need for the prev input data
<instagibbs>
remove virtual stuff :P
* instagibbs
logs off
<instagibbs>
practically speaking, whats the most common pattern that hits it
<instagibbs>
I would have suspects bare multisig?
<sipa>
nothing sane ever hits the increment-vsize-due-to-high-sigops behavior
<sipa>
iirc
SebastianvStaa has quit [Client Quit]
<glozow>
#27591 is perhaps relevant for this discussion? it happened in the wild and a user got super confused that "vsize" was different in the mempool RPC
<luke-jr>
sipa: right, we probably only _need_ it for external transactions (p2p, receiving in wallet)
<Murch>
Hi
<instagibbs>
presumably to reduce cpu DoS, without outright denying the txns?
<sipa>
does anything at all ever care about the non-adjusted vsize?
<luke-jr>
I think non-adjusted is only relevant because BIP141 specified it
<luke-jr>
but not in practice
andrew_m_ has quit [Ping timeout: 255 seconds]
<sipa>
right
<glozow>
I don't think so. We only use non-adjusted when we don't have the prev inputs
<luke-jr>
otoh it's also much simpler to calculate
<luke-jr>
eg, it might not even be possible to get adjusted vsize for historical txs not in our wallet
<sipa>
my suggestion would be to document it; if people care about the consensus-relevant part, there is always weight too, which is more accurate and is unadjusted
<glozow>
👍
<luke-jr>
if not for the complexity of the adjusted-vsize caclulations, I would almost prefer to just say vsize is always adjusted and weight is consensus
<sipa>
yeah the dependency on input data makes it annoying
<_aj_>
could change our rpc fields to say "ajdvsize" when we've taken sigops into account?
<_aj_>
ajdvsize
<_aj_>
jesus
<_aj_>
adjvsize
<sipa>
ETOOMANYVOWELS
<instagibbs>
"aj" hmmm
<luke-jr>
lol
<_aj_>
yeah, pick what my fingers like to type
<josie>
_aj_vsize
SebastianvStaa has quit [Client Quit]
<luke-jr>
well, that's part of why I think vsize vs weight is a better approach, it has a clear distinction
<_aj_>
no, pls don't make me be blamed for legacy sigop limits forever
<glozow>
Would welcome opinions on #27591, the goal there is to clean up the user-facing docs
<_aj_>
i think it looks really clunky, `LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received ...` so a while ago suggested simplifying it, and more recently did a concrete PR #28318 based on that (giving LogDebug(BCLog::NET, "sendtxrc...") instead)
<_aj_>
just wondered if people had thoughts, or if everyone's already agreed on LogPrintLevel(NET, DEBUG, "...") everywhere or ...
andrew_mo_ has joined #bitcoin-core-dev
<sipa>
i don't have a particularly strong opinion, but avoiding clutter sounds nice
<achow101>
less typing is more better?
<fanquake>
It's not completely clear to me what the project is (I don't think it's been active for a while, until 20576 was reopened and description rewritten in the last hour?), what the benefits are, or what needs to be done (guess slightly clearer now with new PR description). The RFC/motivation in #20576 doesn't have much discussion at all.
<fjahr>
I does look clunky but I guess having a shorter alias for LogPrintLevel would also work for me, so no strong opinion...
<stickies-v>
agreed that having clunky logging interface for the most common situations is not great, your approach in 28318 looks much cleaner indeed
<fanquake>
Given it's not a priority project, I don't think it should be a blocker for PRs like 28364, especially when for contributors it's not necessarily clear what should be done
<Murch>
Is jon_atack here perhaps?
<_aj_>
assume not, but we could continue the conversation on the PRs?
<sipa>
i think so
<achow101>
Any other topics to discuss?
<fjahr>
some probably read the previous conversation here, pinheadmz made a new github bot that currently lives in #bitcoin-core-github and now emulates the behavior of the matrix bridge/gh-bot. If people like it could be pointed on this channel as well. But I guess it’s also fine to have a separate channel, at least that also works for me. Just wanted to put it out there as an option.
<_aj_>
fjahr: (yeah, i just made LogDebug an macro replacing with LogPrintLevel in my pr)
<fanquake>
I assume a lot of the necessary changes could also be done with scripted diffs at some point? Rather than making each induvidual PR make changes etc.
<glozow>
so should I just leave #28364 as is? I do personally prefer the less verbose api...
<instagibbs>
if/when direction on logging is set, things can be batch- changed
<lightlike>
fanquake: I agree. I think if we change, we should change all logs of a given category together, and have some high-level plan of what kind of things goes into which severity level.
<glozow>
ok awesome. maybe when we're all together we can have a deeper logging discussion. i will leave it as is and after it's merged, I'll open a PR with the txorphanage changes + tests.
<Murch>
If people feel that it would be a worthwhile change, we could perhaps make the replacement of the logging plumbing a focus in the next release cycle?
<fanquake>
lightlike: yea, the "what belongs in which catergory/severity level" isn't quite clear to me yet, and compunded with the unconditional logging and other things
<Murch>
seems like a big enough thing to have a bunch of people coordinate on it
<instagibbs>
I heard there's a meeting soon(TM), seems like a good topic
<_aj_>
c++20 and cmake could make for a lot of plumbing changes next cycle
andrew_mo_ has quit [Ping timeout: 244 seconds]
<fanquake>
Better oil up your wrench
<MacroFake>
sed -i 's|Span|std::span|g' $( git grep -l Span )
<_aj_>
using Span = std::span :)
<Murch>
Modules are only expected with c++23, right? :(
<Murch>
Well, I guess they’re still out in the future for us then
andrew_mo_ has joined #bitcoin-core-dev
<Murch>
From what I understand that could eventually significantly speed up builds
<sipa>
i'd expect that LTO actually mostly undoes that benefit
<Murch>
What’s LTO?
<sipa>
link-time optimisation
<_aj_>
doesn't LTO seem like it slows down builds a bunch?
<sipa>
yes, it does - but i'd expect that after LTO, modules don't change much
Guyver2 has joined #bitcoin-core-dev
<sipa>
Murch: LTO is where the "compiler" only does a relatively cheap translation step to internal representation in the .o files, and the real conversion to machine language happens at link time (when all the .o get converted together to an executable), when all code is available, rather than just the code from a single .cpp file
<Murch>
Hm, well, compiling is pretty slow for me, even on my desktop, so I was kinda excited
<sipa>
even if we have module support, i doubt we'd actually convert a significant portion of the codebase to be module-based any time soon
<fanquake>
parallelsim in the LTO backends (in newer compilers) has made it somewhat less bad at least
<_aj_>
oh that's neat
<fanquake>
There's also LLVMs ThinLTO, which can get a large portion of the benefits, with much less link time cost
<sipa>
but fair enough, LTO is perhaps not really relevant in this discussion, because at dev time people likely won't use LTO anyway much, only at release build time
andrew_mo_ has quit [Ping timeout: 246 seconds]
<fanquake>
however, we can't really use that unless we switch everything to an LLVM toolchain etc
<sipa>
for context, an important reason why our compilation is slow is because there is a lot of code in header files - making it visible to all modules that use it, leading to better optimization (specifically, better inlining)
<sipa>
both with LTO, and with modules (if my limited understanding serves right) there is much less need for having code in header files
andrew_mo_ has joined #bitcoin-core-dev
<sipa>
and code is headers is especially painful for recompiling after a small change, because it means lots of compilation units change, causing lots of stuff to need recompilation
<sipa>
thinlto allegedly allows incremental recompilation? that sounds pretty crazy
andrew_mo_ has quit [Ping timeout: 255 seconds]
<cfields>
one clear benefit to me is that it lets you put your code where YOU want it, as opposed to where the compiler wants it. Having code in the cpp file as opposed to the header can be a big win for design and cleanliness. But (as sipa said) the inlining you get from keeping everything in the header is often too hard too much to pass up. Our serialization code, for ex, must be header-only in order to keep some pretty complex code compiling down to
<cfields>
(in some cases) a single instruction. With LTO, all of that could be moved out into sane implementation files without sacrificing any speed.
<sipa>
cfields: except for very templaty stuff, of course
bitdex has quit [Quit: = ""]
<cfields>
right
pablomartin4btc has quit [Remote host closed the connection]
pablomartin4btc has joined #bitcoin-core-dev
andrew_mo_ has joined #bitcoin-core-dev
<josie>
hebasto: am I able to rerun the "CI / Win64" ci task? in cirrus I'm able to go in and rerun stuff, but not sure how/if I can do that with the actions one
andrew_mo_ has quit [Ping timeout: 248 seconds]
<sipa>
i believe it is actually not possible with actions
<sipa>
(but i'm not very knowledgable about the topic)
<josie>
bummer :/ the Win64 one is failing a lot recently, with a rerun usually passing
<sipa>
josie: i think i've restarted yours
<sipa>
28122, right?
<josie>
sipa: yep, ty! i see its rerunning now
Guyver2 has left #bitcoin-core-dev [Closing Window]
test_ has quit [Read error: Connection reset by peer]
test_ has joined #bitcoin-core-dev
<_aj_>
i guess we could setup drahtbot to watch for a rerun-failed-ci tag on PRs and trigger a rerun?
andrew_mo_ has joined #bitcoin-core-dev
DarrylTheFiish has quit [Remote host closed the connection]
<josie>
_aj_: or figure out why the CI tasks keeps failing when it shouldnt be and fix it :)