Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] Improve performance when starting nginx with a lot of locations

Maxim Dounin
October 17, 2023 10:04PM
Hello!

On Tue, Oct 17, 2023 at 03:25:41PM +0400, Sergey Kandaurov wrote:

> > On 11 Oct 2023, at 02:56, Maxim Dounin <mdounin@mdounin.ru> wrote:
> >
> > Hello!
> >
> > On Thu, Oct 05, 2023 at 10:51:26AM +0900, Yusuke Nojima wrote:
> >
> >> Thank you for your comment!
> >>
> >>> Could you please provide some more details about the use case,
> >>> such as how locations are used, and what is the approximate number
> >>> of locations being used?
> >>
> >> Our team provides development environments to our company's engineers and QA.
> >> In this environment, engineers and QA can freely create VMs and deploy
> >> applications on them.
> >>
> >> Our nginx has the role of routing requests from the internet to all
> >> applications deployed in this environment.
> >> Additionally, it allows setting IP address restrictions, BASIC
> >> authentication, TLS client authentication, and other configurations
> >> for each application.
> >>
> >> To implement these requirements, we generate a location for each application.
> >> Currently, there are approximately 40,000 locations in our environment.
> >
> > Thank you for the details. Such configuration looks somewhat
> > sub-optimal, but understandable for a development / test
> > environment. And certainly 40k locations is a lot for the sorting
> > algorithm currently used.
> >
> >>> Further, since each location contains configuration for
> >>> all modules, such configurations are expected to require a lot of
> >>> memory
> >>
> >> Each of our nginx processes was consuming 5GB of memory in terms of
> >> resident size.
> >> This is not a problem as our servers have sufficient memory.
> >>
> >>> Rather, I would suggest recursive top-bottom merge sort implementation
> >>> instead, which is much simpler and uses stack as temporary storage
> >>> (so it'll naturally die if there will be a queue which requires
> >>> more space for sorting than we have).
>
> For the record, in my tests on M1 sorting 26^3 locations fit into
> 32k stack size (16k stack size renders the environment unusable).
> Judging by this (unscientific) test, running out of stack should
> not be a practicable issue.

The recursive function uses

ngx_queue_t *q, tail;

on stack, that is, 3 pointers, so it would need more than 1000
recursive calls to overflow 32k stack, which is not expected to
happen unless there are more than 2^1000 locations (and it is
certainly not possible to have more than 2^64 locations on 64-bit
platforms).

For 26^3 locations, sorting would use maximum recursion depth of
15, so would require something like 360 bytes of stack (which is
certainly less than required by other nginx functions).

> >>>
> >>> Please take a look if it works for you:
> >>
> >> I think this implementation is simple and easy to understand.
> >> Although the number of traversals of the list will increase compared
> >> to bottom-up, it will not affect the order.
> >> I believe this will provide sufficient optimization in terms of speed.
> >
> > Thanks for looking. In my limited testing, it is slightly faster
> > than your bottom-up implementation (and significantly faster than
> > the existing insertion sort when many locations are used).
> >
> > Below is the full patch (code unchanged), I'll commit it as soon
> > as some other nginx developer will review it.
> >
> > # HG changeset patch
> > # User Maxim Dounin <mdounin@mdounin.ru>
> > # Date 1696977468 -10800
> > # Wed Oct 11 01:37:48 2023 +0300
> > # Node ID b891840852ee5cc823eee1769d092ab50928919f
> > # Parent cdda286c0f1b4b10f30d4eb6a63fefb9b8708ecc
> > Core: changed ngx_queue_sort() to use merge sort.
> >
> > This improves nginx startup times significantly when using very large number
> > of locations due computational complexity of the sorting algorithm being
>
> due to

Fixed, thnx.

> > used (insertion sort is O(n*n) on average, while merge sort is O(n*log(n))).
>
> nitpicking: E2MANYPARENS
>
> Using a colon might looks better (I don't mind though):
> used: insertion sort is O(n*n) on average, while merge sort is O(n*log(n)).

Thanks, changed.

> > In particular, in a test configuration with 20k locations total startup
> > time is reduced from 8 seconds to 0.9 seconds.
> >
> > Prodded by Yusuke Nojima,
> > https://mailman.nginx.org/pipermail/nginx-devel/2023-September/NUL3Y2FPPFSHMPTFTL65KXSXNTX3NQMK.html
>
> I like the change, please commit.
>
> The thing to keep in mind is that it pessimizes the best case
> of sorted locations, which is O(n) with the insertion sort.
> Though given that both old and new algorithms give relatively
> good numbers for the best case (and it is hard to get a
> noticeable startup delay with very large number of locations
> in practice using merge sort), especially compared to the worst
> case of sorting perfectly reverse sorted locations with the
> insertion sort, I believe this is acceptable.

Sure, theoretically insertion sort's best case is slightly better,
but I don't think it matters: if at all, the difference is
negligible and unlikely can be measured, while noticeable startup
delays with large number of non-sorted locations are easily
observed with insertion sort.

For the record, here are "nginx -t" times with 20k pre-sorted
locations, with insertion sort (before the patch):

0.64 real 0.49 user 0.14 sys
0.59 real 0.51 user 0.07 sys
0.61 real 0.48 user 0.12 sys
0.60 real 0.49 user 0.11 sys
0.61 real 0.51 user 0.09 sys

And with merge sort (after the patch):

0.65 real 0.50 user 0.14 sys
0.63 real 0.49 user 0.14 sys
0.63 real 0.52 user 0.10 sys
0.62 real 0.49 user 0.12 sys
0.67 real 0.55 user 0.11 sys

And ministat(1) for the user times:

$ ministat insert merge
x insert
+ merge
+------------------------------------------------------------------------------+
| * x |
|x * + x + +|
| |_|_____M______A___M_________|A___________________________| |
+------------------------------------------------------------------------------+
N Min Max Median Avg Stddev
x 5 0.48 0.51 0.49 0.496 0.013416408
+ 5 0.49 0.55 0.5 0.51 0.025495098
No difference proven at 95.0% confidence


Thanks for looking, pushed to http://mdounin.ru/hg/nginx/

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] Improve performance when starting nginx with a lot of locations

Yusuke Nojima 366 September 22, 2023 03:00AM

Re: [PATCH] Improve performance when starting nginx with a lot of locations

Maxim Dounin 85 September 29, 2023 11:40PM

Re: [PATCH] Improve performance when starting nginx with a lot of locations

Yusuke Nojima 68 October 04, 2023 09:52PM

Re: [PATCH] Improve performance when starting nginx with a lot of locations

Илья Шипицин 87 October 05, 2023 02:18AM

Re: [PATCH] Improve performance when starting nginx with a lot of locations

Yusuke Nojima 77 October 05, 2023 10:40PM

Re: [PATCH] Improve performance when starting nginx with a lot of locations

Maxim Dounin 65 October 10, 2023 06:58PM

Re: [PATCH] Improve performance when starting nginx with a lot of locations

Yusuke Nojima 70 October 11, 2023 06:34AM

Re: [PATCH] Improve performance when starting nginx with a lot of locations

Sergey Kandaurov 66 October 17, 2023 07:26AM

Re: [PATCH] Improve performance when starting nginx with a lot of locations

Maxim Dounin 88 October 17, 2023 10:04PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 125
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready