Skip to content

Commit 83ec495

Browse files
authored
Remove virtual dispatch from LogData (#13123)
This addresses a performance regression added by #13059. Malformed pre-transaction logging introduced an extra virtual data interface on the normal access log path. That made every completed transaction pay for indirection that is only needed for rare protocol-layer failures. This replaces the virtual hierarchy with a concrete composed TransactionLogData wrapper. This keeps LogAccess using one data object while routing the common HttpSM path through direct getters and falling back to owned pre-transaction data only when no HttpSM exists.
1 parent 060721f commit 83ec495

13 files changed

Lines changed: 1284 additions & 1710 deletions

include/proxy/PreTransactionLogData.h

Lines changed: 7 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424

2525
#pragma once
2626

27-
#include "proxy/logging/TransactionLogData.h"
27+
#include "proxy/Milestones.h"
28+
#include "proxy/hdrs/HTTP.h"
29+
#include "tscore/ink_inet.h"
2830

2931
#include <string>
3032

@@ -36,160 +38,21 @@
3638
* copied request and session metadata needed to emit a best-effort
3739
* transaction log entry for those failures.
3840
*
39-
* Unlike TransactionLogData (which reads from a live HttpSM), this class
40-
* owns its milestones, addresses, and strings because the originating
41-
* stream is about to be destroyed.
41+
* This class owns its milestones, addresses, and strings because the
42+
* originating stream is about to be destroyed.
4243
*/
43-
class PreTransactionLogData : public TransactionLogData
44+
class PreTransactionLogData
4445
{
4546
public:
4647
PreTransactionLogData() = default;
4748

48-
~PreTransactionLogData() override
49+
~PreTransactionLogData()
4950
{
5051
if (owned_client_request.valid()) {
5152
owned_client_request.destroy();
5253
}
5354
}
5455

55-
// ===== Milestones =====
56-
57-
TransactionMilestones const *
58-
get_milestones() const override
59-
{
60-
return &owned_milestones;
61-
}
62-
63-
// ===== Headers =====
64-
65-
HTTPHdr *
66-
get_client_request() const override
67-
{
68-
if (owned_client_request.valid()) {
69-
return const_cast<HTTPHdr *>(&owned_client_request);
70-
}
71-
return nullptr;
72-
}
73-
74-
// ===== Client request URL / path =====
75-
76-
const char *
77-
get_client_req_url_str() const override
78-
{
79-
return owned_url.empty() ? nullptr : owned_url.c_str();
80-
}
81-
int
82-
get_client_req_url_len() const override
83-
{
84-
return static_cast<int>(owned_url.size());
85-
}
86-
const char *
87-
get_client_req_url_path_str() const override
88-
{
89-
return owned_path.empty() ? nullptr : owned_path.c_str();
90-
}
91-
int
92-
get_client_req_url_path_len() const override
93-
{
94-
return static_cast<int>(owned_path.size());
95-
}
96-
97-
// ===== Client addressing =====
98-
99-
sockaddr const *
100-
get_client_addr() const override
101-
{
102-
return &owned_client_addr.sa;
103-
}
104-
sockaddr const *
105-
get_client_src_addr() const override
106-
{
107-
return &owned_client_src_addr.sa;
108-
}
109-
sockaddr const *
110-
get_client_dst_addr() const override
111-
{
112-
return &owned_client_dst_addr.sa;
113-
}
114-
uint16_t
115-
get_client_port() const override
116-
{
117-
return m_client_port;
118-
}
119-
120-
// ===== Squid codes =====
121-
122-
SquidLogCode
123-
get_log_code() const override
124-
{
125-
return m_log_code;
126-
}
127-
SquidHitMissCode
128-
get_hit_miss_code() const override
129-
{
130-
return m_hit_miss_code;
131-
}
132-
SquidHierarchyCode
133-
get_hier_code() const override
134-
{
135-
return m_hier_code;
136-
}
137-
138-
// ===== Transaction identifiers =====
139-
140-
int64_t
141-
get_connection_id() const override
142-
{
143-
return m_connection_id;
144-
}
145-
int
146-
get_transaction_id() const override
147-
{
148-
return m_transaction_id;
149-
}
150-
151-
// ===== Protocol info =====
152-
153-
const char *
154-
get_client_protocol() const override
155-
{
156-
return owned_client_protocol_str.empty() ? nullptr : owned_client_protocol_str.c_str();
157-
}
158-
159-
// ===== Connection flags =====
160-
161-
bool
162-
get_client_connection_is_ssl() const override
163-
{
164-
return m_client_connection_is_ssl;
165-
}
166-
167-
// ===== Server transaction count =====
168-
169-
int64_t
170-
get_server_transact_count() const override
171-
{
172-
return m_server_transact_count;
173-
}
174-
175-
// ===== Fallback fields for pre-transaction logging =====
176-
177-
std::string_view
178-
get_method() const override
179-
{
180-
return owned_method;
181-
}
182-
std::string_view
183-
get_scheme() const override
184-
{
185-
return owned_scheme;
186-
}
187-
std::string_view
188-
get_client_protocol_str() const override
189-
{
190-
return owned_client_protocol_str;
191-
}
192-
19356
// ===== Owned backing storage (public for ProxyTransaction to populate). =====
19457

19558
HTTPHdr owned_client_request;

include/proxy/http/CompletedTransactionLogData.h

Lines changed: 0 additions & 208 deletions
This file was deleted.

include/proxy/logging/Log.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
@section example Example usage of the API
4444
4545
@code
46-
// Populate a LogData (e.g. TransactionLogData or PreTransactionLogData), then:
46+
// Populate a TransactionLogData, then:
4747
LogAccess entry(data);
4848
int ret = Log::access(&entry);
4949
@endcode

0 commit comments

Comments
 (0)