Major rework of Rejection system

[sidju]

Split out the inline modules into their own files.
(reduce length of individual files, more predictable file structure)
Restructure from linked tree into vector.
(increase memory use, but probably reduce allocation latencies)
Add in a Fatal case into Rejections Reasons enum.
(to be converted into a response and returned immediately when occured)

[seanmonstar]

Thanks for the PR! Some of this we can easily merge, and other parts I think need more discussion.

Split out the inline modules into their own files.

Seems like a good call!

Restructure from linked tree into vector. (increase memory use, but probably reduce allocation latencies)

Is this a measured improvement? I assume the common part of rejections is combining them, and the less common is part is searching the tree repeatedly, that's why I optimized for that. I also note that this change increases the size of Rejection, since it now has an enum with a variant of Vec, that is 3 usizes big.

Add in a Fatal case into Rejections Reasons enum. (to be converted into a response and returned immediately when occured)

We've talked a little bit about doing this, but it's not immediately clear that this is the right way forward. I do agree that there's probably something wrong with the current rejection design, since it's such a frequent cause of confusion.

[sidju]

I wrote "probably" specifically because I haven't tested it, but I would expect a reduction in latencies and performance increase. Vec tends to perform better thanks to cache locality and pre-allocation after all. To be honest, though, I mostly rewrote into a vector because the tree confused me and reimplementation is how I make sure I understand. I know there is a lot of discussion on blanket implementing Reply on Result, and async_map. My opinion on this is that this handles the case where a filter suffers a fatal error (such as database being down), and that the filter system is far more flexible if it keeps treating handlers and filters identically. Anyway, give me some pointers on what you want changed or split out and I'll get to it in a week or two.

…

-------- Original Message --------

On 10 Sep 2021, 21:23, Sean McArthur wrote: Thanks for the PR! Some of this we can easily merge, and other parts I think need more discussion. > Split out the inline modules into their own files. Seems like a good call! > Restructure from linked tree into vector. (increase memory use, but probably reduce allocation latencies) Is this a measured improvement? I assume the common part of rejections is combining them, and the less common is part is searching the tree repeatedly, that's why I optimized for that. I also note that this change increases the size of Rejection, since it now has an enum with a variant of Vec, that is 3 usizes big. > Add in a Fatal case into Rejections Reasons enum. (to be converted into a response and returned immediately when occured) We've talked a little bit about doing this, but it's not immediately clear that this is the right way forward. I do agree that there's probably something wrong with the current rejection design, since it's such a frequent cause of confusion. — You are receiving this because you authored the thread. Reply to this email directly, [view it on GitHub](#895 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ADKYNBZR3KLRQ3XLEWVQVH3UBJLL5ANCNFSM5DZEHWTA). Triage notifications on the go with GitHub Mobile for [iOS](https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or [Android](https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub).