Hi, On 2025-04-08 01:17:17 +0200, Daniel Gustafsson wrote: > > On 7 Apr 2025, at 17:43, Andres Freund <and...@anarazel.de> wrote: > > >> + /* > >> + * Hold the process lock to protect writes to process specific memory. Two > >> + * processes publishing statistics do not block each other. > >> + */ > > > > s/specific/process specific/ > > That's what it says though.. isn't it? I might be missing something obvious.
Understandable confusion, not sure what my brain was doing anymore either... > >> +} MemoryContextState; > > > > IMO that's too generic a name for something in a header. > > > >> +} MemoryContextId; > > > > This too. Particularly because MemoryContextData->ident exist but is > > something different. > > Renamed both to use MemoryContextReporting* namespace, which leaves > MemoryContextReportingBackendState at an unwieldly long name. I'm running out > of ideas on how to improve and it does make purpose quite explicit at least. How about MemoryContextReportingBackendState -> MemoryStatsBackendState MemoryContextReportingId -> MemoryStatsContextId MemoryContextReportingSharedState -> MemoryStatsCtl MemoryContextReportingStatsEntry -> MemoryStatsEntry > >> + /* context id starts with 1 */ > >> + entry->context_id = ++(*stats_count); > > > > Given that we don't actually do anything here relating to starting with 1, I > > find that comment confusing. > > Reworded, not sure if it's much better tbh. I'd probably just remove the comment. > > Hm. First I thought we'd leak memory if this second (and subsequent) > > dsa_allocate failed. Then I thought we'd be ok, because the memory would be > > memory because it'd be reachable from memCtxState[idx].memstats_dsa_pointer. > > > > But I think it wouldn't *quite* work, because memCtxState[idx].total_stats > > is > > only set *after* we would have failed. > > Keeping a running total in .total_stats should make the leak window smaller. Why not just initialize .total_stats *before* calling any fallible code? Afaict it's zero-allocated, so the free function should have no problem dealing with the entries that haven't yet been populated/ Greetings, Andres Freund
