Closed tsayukov closed 5 months ago
vectored_io.h
.iovec_ut.cpp
, чтобы они выглядели более аккуратно, плюс добавил дополнительные проверки.tests
(в том числе socket_ut.cpp
и coroutine_ut.cpp
). Остальные тесты из util/network
и library/coroutine
начну переносить после закрытия этого PR (раз уж я начал, логичнее мне и закончить с #176 и #160?).Суммируя, надо было мне изначально создать новый тикет, и уже в нём питчить все серьёзные изменения в TContIOVector
, которые бы помогли решить эту задачу, чтобы не раздувать этот PR. Тоже самое касается остальных сторонних улучшений.
Я починил сборку в CI, сделай rebase
Issue #93
Задача в том, чтобы отказаться от использования libc_compat.
Части libc_compat, которые больше не используются в ydb-cpp-sdk и которые можно спокойно убрать:
getservbyname/
: функцияgetservbyname
и все её помощники. Ранее она была нужна, потому что у llvm msan до сих пор нет interceptor-обёртки для неё (чтобы убедиться в этом, можно написать простой main c ней, скомпилировать с-fsanitize=memory
и черезreadelf -Ws
иgrep
посмотреть таблицу символов), и включение санитайзера могло выдавать ложноположительные результаты.include/ifaddrs/
. Ранееifaddrs.h
добавлялся для Андроида с API level <24; сейчас это не целевая платформа, а также в текущей либе убрали соответствующий c-файл, так что можно не обращать на хедер внимания. Все остальные#include <ifaddrs.h>
в ydb-cpp-sdk включаются через#if
только для Unix-like систем и ссылаются на glibc. Для других платформ подключаются системные хедеры этих платформ с аналогичным функционалом.include/readpassphrase/
.random/
: функцииgetentropy
иgetrandom
. Ранее были нужны из-за их отсутствия в более старых версиях glibc.ubuntu_14/
. Ранее файлы из этой папки нужны были, чтобы на старых версиях Ubuntu ниже 14 находились нужные символы, которых ещё не было в libc.Из остальных хедеров, не считая
include/windows/sys/
, в ydb-cpp-sdk подключается толькоcontrib/libs/libc_compat/string.h
. В нём нас интересуют только функцииstricmp
,strnicmp
,strlcpy
,strlcat
иstrlwr
.strlcpy
,strlcat
иstrlwr
вinclude/ydb-cpp-sdk/library/string_utils/helpers/helpers.h
какsize_t Strlcpy(char*, const char*, size_t)
,size_t Strlcat(char*, const char*, size_t)
,char* ToLower(char*)
. Далее заменить все вызовы на вызовы этих функций.include/ydb-cpp-sdk/util/system/compat.h
определить макросыstricmp
иstrnicmp
, которые в зависимости от платформы будут подставлять соответствующие функции из системного хедераstrings.h
илиstring.h
.src/library/case_insensitive_string/case_insensitive_char_traits.h
привести трейт в соответствии с требованиями. Добавить юнит-тесты.include/windows/sys/
содержит два хедера:queue.h
иuio.h
. Первый,sys/queue.h
больше не включается в ydb-cpp-sdk. Второйsys/uio.h
(в нём нас интересуют две функции:readv
иwritev
) включается при сборке под Windows, таким образом, для остальных систем будет искатьсяsys/uio.h
из glibc, а для Windows выберется хедер вinclude/windows/sys/
, поскольку он добавляется в инклюды libc_compat таргета в cmake. Соответственно, нужно переписатьreadv
иwritev
вsrc/library/coroutine/engine/network.cpp
иsrc/util/network/socket.cpp
так, чтобы они работали и для Windows, и для платформ с glibc.Однако, прежде чем перейти к этой задаче, бросается в глаза следующее (и такое встречается не раз):
Проблема в том, что каст (забудем пока, что это C-style) из указателя к указателю разных типов по стандарту может привести к UB, когда второй указатель попытаются разыменовать (см. [[basic.lval] par.11], они же Type aliasing). На практике компиляторы в редких случаях эксплуатируют этот UB и скорее всего ничего не взорвётся, но я всё равно бы хотел избегать подобных кастов. Обычно такие штуки решают с помощью
std::memcpy
,std::bit_cast
, а ещё в будущем, когда доимплементируют 23 стандарт, можно будет на месте использоватьstd::start_lifetime_as
без копирований.Везде, где вызывался
readv
/writev
/sendmsg
, наблюдался следующий паттерн:TPart
(аналогstd::span
) и его длина;readv
и его друзья), которые читают из или пишут вTPart
ы из этого массива атомарно или хотя бы асинхронно (как в Windows);TPart
в буфере (сдвигают его указатель и уменьшают размер), если обработались только первые сколько-то байтов;У нас уже есть класс-обёртка
TContIOVector
вutil/network/iovec.h
. Поэтому я решил добавить в него буфер, но в этом буфере сразу создавать нужные "C-шные" структуры типаiovec
илиWSABUF
, исключая проблему небезопасных кастов. Подсказывает, как копировать (черезmemcpy
или присваиванием), дополнительный трейтTSpanAdapterTraits
в качестве шаблонного параметра уTContIOVector
, который получился довольно многословным, но в нём я постарался учесть все случаи и сделать максимально обобщённым, а также добавил вспомогательные методы, чтобы можно было бы обращаться к полям "C-шной" структуры в общем случае, заранее не зная, что это за структура. Второй шаблонный параметр уTContIOVector
-- это функтор, вызывающий соответствующую C-шную функцию. Причина, по которой я не захотел передавать функтор, например, в метод, -- не хотелось, чтобы один и тот жеTContIOVector
, изначально предназначенный только для чтения (TPart
ы, допустим, указывают на константные строки, лежащие в readonly памяти), использовали для записи. Возможно, есть решение получше. УTContIOVector
главные методы, которые делают всю работу, -- этоTryProcess
иTryProcessAllBytes
. Соответственно:TSpanAdapterTraits
.TContIOVectorBase
иTContIOVector
(базу сделал на случай возможного расширения, например, если захотим муватьTContIOVector
-писателя вTContIOVector
-читателя).readv
/writev
/sendmsg
.Возможно, стоит подумать о том, чтобы заменить
TPart
(изначально он определён вIOutputStream
) наstd::span<std::bytes>
иstd::span<const std::bytes>
?TPart
плох тем, что он держит в себеconst void*
, а используют его как для чтения, так и для записи. МоемуTSpanAdapterTraits
всё равно, с каким span-подобным объектом работать.Подумал, сильно ли повлияет на производительность сразу выделение буфера (
TTempBuf
до64 * 1024
байтов переиспользует массив из готового списка, соответственно он не будет каждый раз просить новую память в куче). С одной стороны, массив span-ов сам по себе не очень большой, копируется моментально. С другой стороны, на очень больших размерах каждый раз будет выделяться память в куче. С третьей стороны, операции ввода-вывода (даже векторные) наверняка перебьют всё остальное.Вот мой бенчмарк:
Собирался с установленным `libbenchmark-dev`. ```CMake find_package(benchmark REQUIRED) add_executable(iovec_bm iovec_bm.cpp) target_link_libraries(iovec_bm PRIVATE yutil cpp-testing-common benchmark::benchmark_main ) ```Код:
```C++ #include "network/iovec.h" #includeДополнительно
util/memory/tempbuf.*
:inline
у методов с определениями внутри классов, они и так по умолчанию будутinline
.TMP_BUF_LEN
нет никакой необходимости. Можно перенести его значение в static constexpr полеTTempBuf
.char
наstd::byte
? ПосколькуTTempBuf
выделяет область в памяти, которую можно использовать, семантически лучше работать сstd::byte
(или на крайний случай сunsigned char
), который как раз и описывает байты в raw memory. Ко всему прочему, в свежем черновике стандарта убрали, что массивchar
ов неявно создаёт объекты в своём хранилище, см. здесь. Пока отказался, потому что другие классы ожидают отTTempBuf
char
ов, и потребуется много чего исправлять, нагромождаяreinterpret_cast
ами.TPerThreadedBuf
добавить явное выравниваниеalignas(std::max_align_t)
полюData_
. Так как классTTempBuf
в зависимости от запрашиваемого размера выделяет память либо в массивеData_
, либо в куче черезnew
, то нужно выбрать такое выравнивание, которое гарантируется в любом контексте (на стеке или в куче), см. [basic.align] p2. Упомянуть в описании классаTTempBuf
, что over-aligned типы не поддерживаются.TTempBuf
выделяет память в куче. Вспомогательный классTAdditionalStorage
(util/memory/addstorage.h
) выделяет память следующим образом: сначала идёт блок с внутренностямиTTempBuf::TImpl
(указатель, размер, смещение), потом корректно выровненный блок с дополнительной информацией (размер нужной нам памяти) и только затем блок с нужной нам памятью. Начало последнего блока выравнивается поalignas(std::size_t)
, но совсем не факт, что такое выравнивание совпадает с__STDCPP_DEFAULT_NEW_ALIGNMENT__
, а ведьnew
обязано выравнивать память по значению этого макроса, и соответственно наша память, которую потенциально отдаём наружу, должна отвечать таким требования.InfoPtrOffset
, который считает смещение до блока с дополнительной информацией; в методеInfoPtr
просто прибавить это смещение.CombinedSizeOfInstanceWithTInfo
переиспользоватьInfoPtrOffset
и весь результат выровнить по__STDCPP_DEFAULT_NEW_ALIGNMENT__
методомAlignUp
.