Hello, Ben. since you are going to touch 51d code, can you please review the following coverity finding (it thinks there might be null pointer dereference) ?
239 // No need to null check as this has already been carried out in the 240 // calling method 2. returned_null: smp_prefetch_htx returns NULL (checked 35 out of 36 times). [show details <https://scan6.coverity.com/eventId=28590715-1&modelId=28590715-0&fileInstanceId=94351345&filePath=%2Fsrc%2Fhttp_fetch.c&fileStart=172&fileEnd=294> ] 3. var_assigned: Assigning: htx = NULL return value from smp_prefetch_htx. 241 htx = smp_prefetch_htx(smp, chn, 1); 242 4. Condition i < global_51degrees.header_count, taking true branch. 243 for (i = 0; i < global_51degrees.header_count; i++) { 244 name.ptr = (global_51degrees.header_names + i)->area; 245 name.len = (global_51degrees.header_names + i)->data; 246 ctx.blk = NULL; 247 CID 1403847 (#1 of 1): Dereference null return value (NULL_RETURNS)5. dereference: Dereferencing a pointer that might be NULL htx when calling http_find_header. [show details <https://scan6.coverity.com/eventId=28590715-16&modelId=28590715-1&fileInstanceId=94351414&filePath=%2Fsrc%2Fhttp_htx.c&fileStart=54&fileEnd=129> ] 248 if (http_find_header(htx, name, &ctx, 1)) { 249 ws->importantHeaders[ws->importantHeadersCount]. header = ws->dataSet->httpHeaders + i; 250 ws->importantHeaders[ws->importantHeadersCount]. headerValue = ctx.value.ptr; 251 ws->importantHeaders[ws->importantHeadersCount]. headerValueLength = ctx.value.len; 252 ws->importantHeadersCount++; 253 } 254 } 255} 256#endif ср, 23 окт. 2019 г. в 14:40, Ben Shillito <b...@51degrees.com>: > Hi Willy, > > Thanks for letting me know. I will get the work for this scheduled in and > get a patch over to you. > > I agree it looks like a fairly small change for what I'm sure is a > considerable performance increase for large systems. > > Good to see this level of attention to detail when it comes to the > performance of HAProxy. > > Thanks, > > Ben Shillito > Developer > O: +44 1183 287152 > E: b...@51degrees.com > T: @51Degrees > > -----Original Message----- > From: Willy Tarreau [mailto:w...@1wt.eu] > Sent: 23 October 2019 07:04 > To: Ben Shillito <b...@51degrees.com> > Cc: HAProxy <haproxy@formilux.org> > Subject: Possible optimization to 51d in multithread > > Hi Ben, > > after Brian reported the thread performance regression affecting the > pattern matchings in haproxy when relying on the LRU cache, I had a look at > other users of the LRU cache and found that 51d.c is using it with a lock > as well and may also suffer from a lack of linearity with threads. > > You may want to have a look at this patch I just committed to make the > pattern LRU cache per thread : > > 403bfbb130 ("BUG/MEDIUM: pattern: make the pattern LRU cache > thread-local and lockless") > > It's quite straightforward, and if you want to do the same on 51d.c, you > just have to make your lru_tree pointer thread_local and allocate it for > each thread in a small callback registered to be called after threads are > initialized. Same for the call to lru64_destroy(). Then you can remove the > lru lock and gain a lot of scalability. > > I'll let you have a look because I'd rather not break somehing non- > obvious in your code and also because you know better than me how to > benchmark the changes using the real lib and database, but if you need some > help, just let me know. > > Given that it's quite small and simple a change, I'm fine with merging > such a patch for 2.1 even a bit late (but only this, no other changes that > are not bugfixes please). > > Willy > This email and any attachments are confidential and may also be > privileged. If you are not the named recipient, please notify the sender > immediately and do not disclose, use, store or copy the information > contained herein. This is an email from 51Degrees.mobi Limited, 5 Charlotte > Close, Reading. RG47BY. T: +44 118 328 7152; E: i...@51degrees.com; > 51Degrees.mobi Limited t/as 51Degrees. > >