Skip to content

Conversation

@zwoop
Copy link
Contributor

@zwoop zwoop commented Jul 19, 2021

A few comments on this:

  1. I couldn't get the proxy allocation macros to work inside the HttpSM::allocate. But, this patch looks more like what we do for other allocators, making it explicit where we need to allocate a new HttpSM. If someone feels strongly that HttpSM::allocate has to survive, I can see if there's a way to make it work.
  2. Bryan requested that I verify that the allocation of the HttpSM happens on the ET_NET thread even with accept threads, and I did verify that and it does. So the HttpSM gets allocated and released on the same ET_NET thread.

@zwoop zwoop added this to the 10.0.0 milestone Jul 19, 2021
@zwoop zwoop requested a review from bryancall July 19, 2021 16:29
@zwoop zwoop self-assigned this Jul 19, 2021
@zwoop zwoop marked this pull request as draft July 19, 2021 16:29
@bryancall
Copy link
Contributor

@zwoop can you post the performance numbers of having a proxy allocator for the HttpSM vs not?

@zwoop
Copy link
Contributor Author

zwoop commented Jul 20, 2021

I've tried this on master, 9.1.x and 9.0.x branches, and on the 9.0.x branch, it will assert always. 9.1.x or master does not have this problem, I think something changed around either the class allocators or the constructor / destructors for the HttpSM.

@zwoop
Copy link
Contributor Author

zwoop commented Jul 20, 2021

The performance increase is not tremendous, about 5%. It's possible it make more of a difference on other hardware platforms, where the costs of the atomic operations on the class allocator causes more contention.

@zwoop zwoop marked this pull request as ready for review July 21, 2021 17:53
@zwoop zwoop merged commit 4682e8d into apache:master Jul 21, 2021
@zwoop zwoop deleted the MakeHttpSMProxyAllocated branch July 21, 2021 19:18
zwoop added a commit that referenced this pull request Aug 5, 2021
@zwoop
Copy link
Contributor Author

zwoop commented Aug 5, 2021

Cherry-picked to v9.1.x branch.

@zwoop zwoop modified the milestones: 10.0.0, 9.1.0 Aug 5, 2021
ezelkow1 added a commit to ezelkow1/trafficserver that referenced this pull request Mar 7, 2022
ezelkow1 added a commit to ezelkow1/trafficserver that referenced this pull request Mar 7, 2022
…he#6241)"

This reverts commit b24f62f.

Revert "Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator (apache#7584)"

This reverts commit a0dd3c2.

Revert "Changes HttpSM to be Proxy Allocated (apache#8082)"

This reverts commit 6806795.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants