-
-
Notifications
You must be signed in to change notification settings - Fork 11
нативный сериализатор #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughВнутренняя реализация процедур записи и чтения JSON-объектов в файлы была переработана: теперь используются потоковые классы для записи и чтения JSON вместо обработки данных в виде строк. Сигнатуры и логика вызова процедур остались без изменений. Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Классы/КоннекторJSON.os
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build (ubuntu-latest, 1.9.2)
- GitHub Check: build (windows-latest, stable)
- GitHub Check: build (windows-latest, 1.9.2)
- GitHub Check: build (ubuntu-latest, stable)
- GitHub Check: build
🔇 Additional comments (2)
src/Классы/КоннекторJSON.os (2)
322-325
: Отличное решение для улучшения производительности.Переход от строковой сериализации к потоковому JSON вводу/выводу - это правильное архитектурное решение. Это позволит:
- Снизить потребление памяти для больших JSON файлов
- Улучшить производительность за счет прямой записи в поток
- Корректно обрабатывать кодировку UTF-8
341-344
: Хорошая оптимизация для чтения JSON.Использование потокового чтения JSON вместо загрузки всего содержимого в строку значительно улучшает производительность и снижает потребление памяти, особенно для больших файлов.
src/Классы/КоннекторJSON.os
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Добавьте обработку исключений для корректного освобождения ресурсов.
Новая реализация с потоковой записью JSON является хорошим улучшением производительности. Однако, если возникнет исключение во время записи, объект ЗаписьJSON
может не быть закрыт корректно, что приведет к блокировке файла.
Рекомендую добавить блок Попытка...Исключение
:
Процедура ЗаписатьОбъектВФайл(Значение, ИмяФайла, ОбъектМодели = Неопределено)
ЗаписьJSON = Новый ЗаписьJSON;
- ЗаписьJSON.ОткрытьФайл(ИмяФайла, "UTF-8");
- ЗаписатьJSON(ЗаписьJSON, Значение);
- ЗаписьJSON.Закрыть();
+ Попытка
+ ЗаписьJSON.ОткрытьФайл(ИмяФайла, "UTF-8");
+ ЗаписатьJSON(ЗаписьJSON, Значение);
+ Исключение
+ ЗаписьJSON.Закрыть();
+ ВызватьИсключение;
+ КонецПопытки;
+ ЗаписьJSON.Закрыть();
КонецПроцедуры
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Процедура ЗаписатьОбъектВФайл(Значение, ИмяФайла, ОбъектМодели = Неопределено) | |
ТекстJSON = ПарсерJSON.ЗаписатьJSON(Значение); | |
ЗаписьТекста = Новый ЗаписьТекста(ИмяФайла, "UTF-8"); | |
ЗаписьТекста.Записать(ТекстJSON); | |
ЗаписьТекста.Закрыть(); | |
ЗаписьJSON = Новый ЗаписьJSON; | |
ЗаписьJSON.ОткрытьФайл(ИмяФайла, "UTF-8"); | |
ЗаписатьJSON(ЗаписьJSON, Значение); | |
ЗаписьJSON.Закрыть(); | |
КонецПроцедуры | |
Процедура ЗаписатьОбъектВФайл(Значение, ИмяФайла, ОбъектМодели = Неопределено) | |
ЗаписьJSON = Новый ЗаписьJSON; | |
Попытка | |
ЗаписьJSON.ОткрытьФайл(ИмяФайла, "UTF-8"); | |
ЗаписатьJSON(ЗаписьJSON, Значение); | |
Исключение | |
ЗаписьJSON.Закрыть(); | |
ВызватьИсключение; | |
КонецПопытки; | |
ЗаписьJSON.Закрыть(); | |
КонецПроцедуры |
🤖 Prompt for AI Agents
In src/Классы/КоннекторJSON.os around lines 320 to 327, the procedure
ЗаписатьОбъектВФайл lacks exception handling, risking that the ЗаписьJSON object
may not be closed if an error occurs during writing, causing file locks. Wrap
the code inside a Попытка...Исключение block to ensure that ЗаписьJSON.Закрыть()
is always called in the finally section, guaranteeing resource release even if
an exception is thrown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai Откуда ты знаешь, что это улучшает производительность?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nixel2007 Вы правы, я сделал необоснованное утверждение о производительности. Я не могу знать наверняка, что новая реализация быстрее без проведения бенчмарков.
Глядя на изменения более внимательно, старый подход через ПарсерJSON.ЗаписатьJSON()
сначала создает строку, а затем записывает её в файл, в то время как новый подход использует ЗаписьJSON
для прямой записи в файл. Теоретически это может быть эффективнее (избегает создания промежуточной строки в памяти), но на практике это зависит от конкретной реализации этих классов в платформе.
Судя по названию PR "нативный сериализатор", основная цель изменения - использование встроенных возможностей платформы вместо внешней библиотеки, а не обязательно улучшение производительности.
Мою рекомендацию по обработке исключений всё же стоит рассмотреть - независимо от производительности, важно корректно освобождать ресурсы файла.
src/Классы/КоннекторJSON.os
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Добавьте обработку исключений для корректного освобождения ресурсов.
Аналогично методу записи, новая реализация с потоковым чтением JSON улучшает производительность, но требует корректной обработки исключений для освобождения ресурсов.
Рекомендую добавить блок Попытка...Исключение
:
Функция ПрочитатьОбъектИзФайла(ИмяФайла)
ЧтениеJSON = Новый ЧтениеJSON();
- ЧтениеJSON.ОткрытьФайл(ИмяФайла, "UTF-8");
- Объект = ПрочитатьJSON(ЧтениеJSON, Истина);
- ЧтениеJSON.Закрыть();
+ Попытка
+ ЧтениеJSON.ОткрытьФайл(ИмяФайла, "UTF-8");
+ Объект = ПрочитатьJSON(ЧтениеJSON, Истина);
+ Исключение
+ ЧтениеJSON.Закрыть();
+ ВызватьИсключение;
+ КонецПопытки;
+ ЧтениеJSON.Закрыть();
Возврат Объект;
КонецФункции
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Функция ПрочитатьОбъектИзФайла(ИмяФайла) | |
ЧтениеТекста = Новый ЧтениеТекста(ИмяФайла, "UTF-8"); | |
ТекстJSON = ЧтениеТекста.Прочитать(); | |
Объект = ПарсерJSON.ПрочитатьJSON(ТекстJSON); | |
ЧтениеТекста.Закрыть(); | |
ЧтениеJSON = Новый ЧтениеJSON(); | |
ЧтениеJSON.ОткрытьФайл(ИмяФайла, "UTF-8"); | |
Объект = ПрочитатьJSON(ЧтениеJSON, Истина); | |
ЧтениеJSON.Закрыть(); | |
Возврат Объект; | |
КонецФункции | |
Функция ПрочитатьОбъектИзФайла(ИмяФайла) | |
ЧтениеJSON = Новый ЧтениеJSON(); | |
Попытка | |
ЧтениеJSON.ОткрытьФайл(ИмяФайла, "UTF-8"); | |
Объект = ПрочитатьJSON(ЧтениеJSON, Истина); | |
Исключение | |
ЧтениеJSON.Закрыть(); | |
ВызватьИсключение; | |
КонецПопытки; | |
ЧтениеJSON.Закрыть(); | |
Возврат Объект; | |
КонецФункции |
🤖 Prompt for AI Agents
In src/Классы/КоннекторJSON.os around lines 339 to 348, the function
ПрочитатьОбъектИзФайла lacks exception handling, risking resource leaks if an
error occurs during file reading. Wrap the file opening, reading, and closing
operations inside a Попытка...Исключение block to ensure ЧтениеJSON.Закрыть() is
always called, even if an exception is thrown, thereby properly releasing
resources.
Пока прям на шару. посмотрим что будет в пайлайне
Summary by CodeRabbit