Repository navigation
Conversation
| "priority": int | ||
| "fallback": (???) # Нужен ли fallback | ||
|
|
||
| "device": |
There was a problem hiding this comment.
А почму устройство никак не свзянос оптимизациями? Предположим, я хочу сранвить, как разные параметры влияют на производительность.
gsvgit
left a comment
There was a problem hiding this comment.
Не вполне опнятне сценарий использования конфига. Кажется, один из возможных такой. В файле (или в нескольких) описываются списки со сконфигурированными устройствами. При зввпуске пользовательского приложения ему передаётся конфиг и (опционально) идентификатор конфигурации, на которой запутиться. Если конфигурация не указана, выбирается дефолтная (решить, как её выбирать).
|
|
||
|
|
||
| ## Итог: | ||
| По результатам сравнения, вероятно, лучшим вариантом будет использование `CLI11` и `nlohmann/json`, так как они, в отличие от `Boost.program_options`, не требуют зависимостей и являются header-only. |
There was a problem hiding this comment.
Меньше issues и как будто активность поменьше. Я помню, Вы говорнили, что это дискуссионная характеристика. Всетаки config-cxx взять?
| config: | ||
|
|
||
| "platform": | ||
| "selection": |
There was a problem hiding this comment.
Не понимаю по названию, что это за блок и почему он так назван.
There was a problem hiding this comment.
способ выбора платформы: можно по названию пользователя, по официальному имени и по вендору. Мне кажется, это естественные поля
| "priority": int | ||
| "fallback": (???) # Нужен ли fallback | ||
| "device": | ||
| "selection": |
| "official_name": string | ||
| "type": "gpu" | "cpu" | ||
| "index": int | ||
| "priority": int |
There was a problem hiding this comment.
Кажется, про очериди ничего нету.
There was a problem hiding this comment.
Я вот про эти очереди: Количество очередей: сейчас создается только 1 очередь При чём тут priority?
There was a problem hiding this comment.
тогда я не очень понимаю, что в контексте спла значит очередь и какие у нее параметры
| allocator_size = 1048576 | ||
| ``` | ||
|
|
||
| Формата ini. |
There was a problem hiding this comment.
Откуда тут ini. Ещё и на линуксах. Ещё и в 26 году...
There was a problem hiding this comment.
Сейчас здесь примеры того, как могут выглядеть конфигурационные файлы в разных форматах. Исходный формат зависит от того, какую библиотеку для парсинга файлов выберем
Кроме этого все впорядке?
Можно уже начать писать сам механизм?
Если комментарии выше больше не актуальны, пометьте, пожалуйста, их как resolved
| Файл конфигурации, предоставляющийся библиотекой, имеет следующую структуру: | ||
| ```json | ||
| { | ||
| "current_config": "default", |
There was a problem hiding this comment.
Strange name. Current will be selected by user at the start.
There was a problem hiding this comment.
Почему странное? Пользователь может и не выбирать конфигурацию, но чтобы программа не падала этот параметр имеет дефолтное значение. Если пользователь выберет конфигурацию явно, то оно переопределится значением пользователя
Если пользователь не выбрал конфигурацию, то "текущая конфигурация" - дефолтная, если выбрал, "текущая конфигурация" - выбранная пользоватеоем. На мой взгляд название отражает смысл
| }, | ||
|
|
||
| "debug_gpu0": { | ||
| "extends": ["debug", "gpu0"] |
There was a problem hiding this comment.
Can extends be used not for list of configurations, but to redefine fields for particular configuration? As a way to say "I want to set profiling = true for gpu0 config".
There was a problem hiding this comment.
Да.
Можно сделать так:
"current_config": "gpu_profiling_true",
"gpu0": {
"backend": "by_index",
"platform_index": 0,
"device_index": 0,
"if_gpu_unavailable": "abort",
"queues_count": 1,
"profiling": false
"allocator_type": "general",
"verbosity": 3
},
"gpu_profiling_true": {
"extends": ["gpu0"],
"profiling": true
}
В этом случае параметры конфигурации "gpu0" переопределятся параметрами конфигурации "gpu_profiling_true" ( "profiling": false станет true)
|
|
||
| "configs": { | ||
| "default": { | ||
| "backend": "gpu", // переопределен |
There was a problem hiding this comment.
Why? I see only one configuration in the file.
There was a problem hiding this comment.
Конфигурация "gpu0" включает конфигурацию "default", как базовую. Ее параметры будут переопределены параметрами конфигурации "gpu0". Поэтому "backend": "gpu" переопределяется "backend": "by_index". В extends можно записывать конфигурации, которые находятся в конфигурационных файлах с меньшим приоритетом. (в системном файле пользователь может указать конфигурацию из дефолтного файла, в юзер файле - конфигурацию из системного)
Например:
1 Файл, который идет с библиотекой имеет конфигурацию "default"
2 Файл, созданный пользователем имеет конфигурацию "gpu0"
В поле extends конфигурации "gpu0" пользователь может добавить конфигурацию "default", несмотря на то, что она в дефолтном файле бибилиотеки, а не в файле, который создал пользователь
Это сделано для того, чтобы не нудно было дублировать параметры
|
|
||
| - name: Run tests | ||
| env: | ||
| SPLA_CONFIG_FILE: ${{ github.workspace }}/.github/workflows/ci_spla_conf.json |
There was a problem hiding this comment.
Нет. Так делать в описываемом сценарии нельзя. Подумайте, почему.
| - name: Check for tests results | ||
| run: | | ||
| ! grep -qi "FAILED" ${{ env.test-file }} | ||
| ! grep -qE "\[ FAILED \]" ${{ env.test-file }} |
| @@ -0,0 +1,121 @@ | |||
| # Сравнение библиотек для создания механизма конфигурирования | |||
| m_max_wgs = m_device.getInfo<CL_DEVICE_MAX_WORK_GROUP_SIZE>(); | ||
| m_max_local_mem = m_device.getInfo<CL_DEVICE_LOCAL_MEM_SIZE>(); | ||
| m_addr_align = m_device.getInfo<CL_DEVICE_MEM_BASE_ADDR_ALIGN>() / 8;// from bits to bytes | ||
| m_addr_align = m_device.getInfo<CL_DEVICE_MEM_BASE_ADDR_ALIGN>() / 8; |
There was a problem hiding this comment.
They seemed unnecessary. You mentioned that code comments aren't exactly a best practice
There was a problem hiding this comment.
Except cases when it is hard to extract some information from code directly.
| m_wave_size = 64; | ||
| m_is_amd = true; | ||
|
|
||
| // Likely, it is an integrated amd device |
|
|
||
| for (int i = 0; i < count; i++) { | ||
| cl_command_queue_properties properties = 0; | ||
| #ifndef SPLA_RELEASE |
There was a problem hiding this comment.
Queue profiling can now be explicitly specified
| ->envname("SPLA_PROFILING"); | ||
|
|
||
| app.add_option("-sa,--spla-allocator-type", cfg.allocator_type, | ||
| "Allocator type:\n" |
There was a problem hiding this comment.
Yet another description of flags/ Why?
There was a problem hiding this comment.
Files src/opencl/cl_configure.cpp and src/opencl/cl_configure.hpp were moved to src/core, but I forgot to delete these ones
| MessageCallback m_callback; | ||
|
|
||
| MessageCallback m_callback; | ||
| int m_verbosity = 3; |
| @@ -0,0 +1,121 @@ | |||
| # Comparison of libraries for creating a configuration mechanism | |||
| print(f"pyspla: [{decoded_file}:{line}] {_status_mapping[status]}: {decoded_msg}") | ||
|
|
||
|
|
||
| def filter_spla_args(argv): |
There was a problem hiding this comment.
Can you use standard Python any package for arguments processing?
| auto* lib = spla::Library::get(); | ||
| if (!lib) return SPLA_STATUS_ERROR; | ||
|
|
||
| if (spla::config_final.backend.has_value() && |
There was a problem hiding this comment.
What this condition means? Why you check manually only this condition?
| LOG_MSG(Status::DeviceNotFound, "index out of list of available devices"); | ||
| return Status::DeviceNotFound; | ||
| } | ||
| m_platform.getDevices(CL_DEVICE_TYPE_ALL, &available_devices); |
There was a problem hiding this comment.
Why we request all types always instead of GPU?
| m_max_wgs = m_device.getInfo<CL_DEVICE_MAX_WORK_GROUP_SIZE>(); | ||
| m_max_local_mem = m_device.getInfo<CL_DEVICE_LOCAL_MEM_SIZE>(); | ||
| m_addr_align = m_device.getInfo<CL_DEVICE_MEM_BASE_ADDR_ALIGN>() / 8;// from bits to bytes | ||
| m_addr_align = m_device.getInfo<CL_DEVICE_MEM_BASE_ADDR_ALIGN>() / 8; |
There was a problem hiding this comment.
Except cases when it is hard to extract some information from code directly.
| return spla_Configure(static_cast<int>(args.size()), args.data()); | ||
| } | ||
|
|
||
| void print_usage() { |
There was a problem hiding this comment.
Really? C++ has no library that can automatically infer usage from registered CLI arguments?
|
|
||
| if (SPLA_BUILD_OPENCL) | ||
| if (SPLA_TARGET_MACOSX) | ||
| message(STATUS "Add standard Apple OpenCL package") |
|
|
||
| add_library(OpenCLHeaders INTERFACE) | ||
| add_library(OpenCL::Headers ALIAS OpenCLHeaders) | ||
| target_include_directories(OpenCLHeaders INTERFACE ${OpenCL_INCLUDE_DIRS}) |
| endif () | ||
|
|
||
|
|
||
| message(STATUS "Add Khronos OpenCL HPP bindings") |
| if (SPLA_BUILD_OPENCL) | ||
| target_link_libraries(spla PUBLIC OpenCL) | ||
| target_link_libraries(spla PUBLIC OpenCL::HeadersCpp) | ||
| target_compile_definitions(spla PUBLIC CL_TARGET_OPENCL_VERSION=120) |
There was a problem hiding this comment.
What the reason of these changes?
Comparison of libraries for creating a configuration mechanism
All information is taken from the repositories of the corresponding projects. Access date: 21.04.26.
Comparison criteria
Comparison of command line parsers (CLI)
File parsers
Comparison of universal tools
Conclusion:
Based on the comparison results, the best choice is likely to use
CLI11andnlohmann/json, since, unlikeBoost.program_options, they require no dependencies and are header-only.