[squid-dev] [PATCH] Http::Stream ID numbering

[Alex Rousskov]

On 02/03/2016 04:29 AM, Amos Jeffries wrote:
> Okay, given a week with no response I merged the non-logic shuffling part.

I have not seen what you have committed (too busy dealing with trunk
bugs and other issues), but, FWIW, based on past experience, there may
be a fundamental disagreement regarding what constitutes no-logic
changes. For example, creating new classes and moving old classes into
new namespaces is often _not_ a no-logic change. In fact, it is one of
the most difficult-to-get-right changes! Whether the compiled code is
about the same after the change is irrelevant.

Again, I am _not_ saying you have just committed something you should
not have. I am only saying that our definitions probably differ. How to
deal with that is your call. Right now, I cannot offer any practical
solution to resolve those differences or protect us from their bad side
effects.


>>>> Pipeline class is updated to use the ID number to manage its contents
>>>> rather than Pointer value matching. It is also updated to drop the
>>>> HTTP/1 specific assumptions within the Pipeline implementation. As a
>>>> behavioural requirement the sequential flow is now left for the Server
>>>> and ClientHttpRequest Jobs to ensure correctness.
>>>
>>>
>>> The new Pipeline::popById() method is a bad idea IMO: Linear search is
>>> wrong for both HTTP/1 and HTTP/2.
>>>
>>
>> Do you have a better algorithm? it needs to pop using only an ID (HTTP/2
>> limit due to framing) and works on linear/list storage (optimal for
>> HTTP/1 sequentialism).

I am sure I can create a solution, but I was hoping to avoid doing that.
I can start by suggesting that if what you are saying is true, then you
probably need to either

* configure Pipeline to use one of the two different storage structures
with the same API (optimal but often more difficult to design right) or

* always use one Pipeline storage structure with two indices (worse
performance, more RAM, and messier code but possibly easier to shovel
into Squid).

For the first bullet, please note HTTP/1 can internally do a
simple/efficient "pop(void)" and only then check the popped ID, even
though its public API can be exactly the same "removeById()" API used by
HTTP/2.

BTW, "Pipeline" does not sound like the right name for a class managing
mostly independent components each of which can be removed at any time
without significant effect on others (as is the case with HTTP/2 streams).


HTH,

Alex.