You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/07/16 07:35:08 UTC
[GitHub] [trafficserver] masaori335 opened a new pull request #8069: Avoid heap allocation in TSMimeHdrField* APIs
masaori335 opened a new pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069
An alternative approach of #8052 following @zwoop's suggestion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] sudheerv commented on pull request #8069: Add an option to avoid heap allocation in TSMimeHdrField* APIs
Posted by GitBox <gi...@apache.org>.
sudheerv commented on pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069#issuecomment-1011567895
Can we not use `ProxyAllocator` like @zwoop recommended in #8052 which abstracts away the memory management from the Plugins (and less boiler plate code) as well? If breaking compatibility was a concern (not entirely sure if it'd be a necessarily an incompatible change to switch to ProxyAllocator, but maybe the concern is that it might require/allocate a significantly more memory?), could we not use a build control to switch to new behavior and eventually make that default at some point?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on pull request #8069: Add an option to avoid heap allocation in TSMimeHdrField* APIs
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069#issuecomment-882178343
The new commit fixed compile errors of lua, experimental, and example plugins.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #8069: Avoid heap allocation in TSMimeHdrField* APIs
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069#issuecomment-881917008
There's a few compile failures here, Lua plugin needs changes etc.
```
CC lua/tslua_la-ts_lua_client_response.lo
/homer/leif/apache/trafficserver/plugins/lua/ts_lua_client_response.c: In function ‘ts_lua_client_response_set_error_resp’:
/homer/leif/apache/trafficserver/plugins/lua/ts_lua_client_response.c:425:15: error: too few arguments to function ‘TSMimeHdrFieldFind’
425 | field_loc = TSMimeHdrFieldFind(http_ctx->client_response_bufp, http_ctx->client_response_hdrp, TS_MIME_FIELD_TRANSFER_ENCODING,
| ^~~~~~~~~~~~~~~~~~
In file included from /homer/leif/apache/trafficserver/plugins/lua/ts_lua_common.h:30,
from /homer/leif/apache/trafficserver/plugins/lua/ts_lua_util.h:21,
from /homer/leif/apache/trafficserver/plugins/lua/ts_lua_client_response.c:19:
/homer/leif/apache/trafficserver/include/ts/ts.h:964:14: note: declared here
964 | tsapi TSMLoc TSMimeHdrFieldFind(TSMBuffer bufp, TSMLoc hdr, const char *name, int length, TSHdrHandle *handle);
| ^~~~~~~~~~~~~~~~~~
gmake[2]: [Makefile:5837: lua/tslua_la-ts_lua_client_response.lo] Error 1 (ignored)
CCLD lua/tslua.la
libtool: error: 'lua/tslua_la-ts_lua_client_response.lo' is not a valid libtool object
gmake[2]: [Makefile:5030: lua/tslua.la] Error 1 (ignored)
CXX experimental/slice/HttpHeader.lo
/homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc: In member function ‘bool HttpHeader::hasKey(const char*, int) const’:
/homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc:140:75: error: too few arguments to function ‘tsapi_mloc* TSMimeHdrFieldFind(TSMBuffer, TSMLoc, const char*, int, TSHdrHandle*)’
140 | TSMLoc const locfield(TSMimeHdrFieldFind(m_buffer, m_lochdr, key, keylen), nullptr);
| ^
In file included from /homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.h:35,
from /homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc:19:
/homer/leif/apache/trafficserver/include/ts/ts.h:964:14: note: declared here
964 | tsapi TSMLoc TSMimeHdrFieldFind(TSMBuffer bufp, TSMLoc hdr, const char *name, int length, TSHdrHandle *handle);
| ^~~~~~~~~~~~~~~~~~
/homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc:140:85: error: expression list treated as compound expression in initializer [-fpermissive]
140 | TSMLoc const locfield(TSMimeHdrFieldFind(m_buffer, m_lochdr, key, keylen), nullptr);
| ^
/homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc: In member function ‘bool HttpHeader::setKeyVal(const char*, int, const char*, int, int)’:
/homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc:212:72: error: too few arguments to function ‘tsapi_mloc* TSMimeHdrFieldFind(TSMBuffer, TSMLoc, const char*, int, TSHdrHandle*)’
212 | TSMLoc locfield(TSMimeHdrFieldFind(m_buffer, m_lochdr, keystr, keylen), nullptr);
| ^
In file included from /homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.h:35,
from /homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc:19:
/homer/leif/apache/trafficserver/include/ts/ts.h:964:14: note: declared here
964 | tsapi TSMLoc TSMimeHdrFieldFind(TSMBuffer bufp, TSMLoc hdr, const char *name, int length, TSHdrHandle *handle);
| ^~~~~~~~~~~~~~~~~~
/homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc:212:82: error: expression list treated as compound expression in initializer [-fpermissive]
212 | TSMLoc locfield(TSMimeHdrFieldFind(m_buffer, m_lochdr, keystr, keylen), nullptr);
| ^
/homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc: In member function ‘bool HttpHeader::setKeyTime(const char*, int, time_t)’:
/homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc:268:72: error: too few arguments to function ‘tsapi_mloc* TSMimeHdrFieldFind(TSMBuffer, TSMLoc, const char*, int, TSHdrHandle*)’
268 | TSMLoc locfield(TSMimeHdrFieldFind(m_buffer, m_lochdr, keystr, keylen), nullptr);
| ^
In file included from /homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.h:35,
from /homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc:19:
/homer/leif/apache/trafficserver/include/ts/ts.h:964:14: note: declared here
964 | tsapi TSMLoc TSMimeHdrFieldFind(TSMBuffer bufp, TSMLoc hdr, const char *name, int length, TSHdrHandle *handle);
| ^~~~~~~~~~~~~~~~~~
/homer/leif/apache/trafficserver/plugins/experimental/slice/HttpHeader.cc:268:82: error: expression list treated as compound expression in initializer [-fpermissive]
268 | TSMLoc locfield(TSMimeHdrFieldFind(m_buffer, m_lochdr, keystr, keylen), nullptr);
| ^
gmake[2]: [Makefile:6140: experimental/slice/HttpHeader.lo] Error 1 (ignored)
CXXLD experimental/slice/slice.la
libtool: error: 'experimental/slice/HttpHeader.lo' is not a valid libtool object
gmake[2]: [Makefile:4691: experimental/slice/slice.la] Error 1 (ignored)
CXX experimental/tls_bridge/tls_bridge.lo
/homer/leif/apache/trafficserver/plugins/experimental/tls_bridge/tls_bridge.cc: In function ‘void Hdr_Remove_Field(TSMBuffer, TSMLoc, ts::TextView)’:
/homer/leif/apache/trafficserver/plugins/experimental/tls_bridge/tls_bridge.cc:51:96: error: too few arguments to function ‘tsapi_mloc* TSMimeHdrFieldFind(TSMBuffer, TSMLoc, const char*, int, TSHdrHandle*)’
51 | if (TS_NULL_MLOC != (field_loc = TSMimeHdrFieldFind(mbuf, hdr_loc, field.data(), field.size()))) {
| ^
In file included from /homer/leif/apache/trafficserver/plugins/experimental/tls_bridge/tls_bridge.cc:16:
/homer/leif/apache/trafficserver/include/ts/ts.h:964:14: note: declared here
964 | tsapi TSMLoc TSMimeHdrFieldFind(TSMBuffer bufp, TSMLoc hdr, const char *name, int length, TSHdrHandle *handle);
| ^~~~~~~~~~~~~~~~~~
gmake[2]: [Makefile:6140: experimental/tls_bridge/tls_bridge.lo] Error 1 (ignored)
CXXLD experimental/tls_bridge/tls_bridge.la
libtool: error: 'experimental/tls_bridge/tls_bridge.lo' is not a valid libtool object
gmake[2]: [Makefile:4811: experimental/tls_bridge/tls_bridge.la] Error 1 (ignored)
gmake[2]: Leaving directory '/Builds/Release/plugins'
gmake[1]: Leaving directory '/Builds/Release/plugins'
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on pull request #8069: Avoid heap allocation in TSMimeHdrField* APIs
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069#issuecomment-881242502
The first commit changes only `TSMimeHdrFieldFind` to see the shape of the code. Many files are changed by below sed.
```
find . -name "*.c" | xargs -n 1 sed -i '' "s/TSMimeHdrFieldFind(\(.*\));/TSMimeHdrFieldFind(\1, NULL);/g"
find . -name "*.h" -o -name "*.cc" | xargs -n 1 sed -i '' "s/TSMimeHdrFieldFind(\(.*\));/TSMimeHdrFieldFind(\1, nullptr);/g"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #8069: Add an option to avoid heap allocation in TSMimeHdrField* APIs
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069#issuecomment-882649404
Looks good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #8069: Add an option to avoid heap allocation in TSMimeHdrField* APIs
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069#issuecomment-882649404
Looks good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #8069: Add an option to avoid heap allocation in TSMimeHdrField* APIs
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069#issuecomment-882649404
Looks good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #8069: Add an option to avoid heap allocation in TSMimeHdrField* APIs
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069#issuecomment-944416520
[approve ci]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on pull request #8069: Add an option to avoid heap allocation in TSMimeHdrField* APIs
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on pull request #8069:
URL: https://github.com/apache/trafficserver/pull/8069#issuecomment-887064811
As noted on the other PR, I would prefer an approach that returns the struct rather than taking a pointer. There's no state in the struct so clean up is unncessary. A full struct would only be 16-24 bytes which is not excessive for a return. In fact, looking at the generated code, the compiler allocates the return struct on the stack and passes a pointer to it to the function, making that just as efficient (if not more so) than explicitly passing the pointer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org