Skip to content

experiment: Оптимизация сабсистем#8476

Open
dageavtobusnick wants to merge 37 commits intoss220-space:master220from
dageavtobusnick:optimisation_2
Open

experiment: Оптимизация сабсистем#8476
dageavtobusnick wants to merge 37 commits intoss220-space:master220from
dageavtobusnick:optimisation_2

Conversation

@dageavtobusnick
Copy link
Collaborator

@dageavtobusnick dageavtobusnick commented Jan 28, 2026

Что этот ПР делает

Добавляет проверки на тик мастера в http и events сабсистемы.
Оптимизирует генерацию иконок для лут панели за счет иконфорджа.
Оптимизирует спаны за счет замены тех, что вызываются в рантайме с конкатинации на интерполяцию.
Оптимизирует количество списков строк за счет их кеширования
Оптимизирует операции возведения во вторую и третью степень путем замены на умножение.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces optimizations for subsystems and icon generation. However, a critical security concern has been identified: a high-severity path traversal vulnerability exists in the new icon generation logic. This is due to the use of unsanitized atom names to construct file paths on the server, which could lead to sensitive file overwrites or the creation of malicious content in arbitrary locations. Beyond the security vulnerability, the review also highlighted a critical issue with an incorrect stage definition for the event subsystem in code/controllers/subsystem/events.dm, which will cause only one processing stage to execute. Recommendations include correcting the random color generation loop in code/__HELPERS/icons.dm which performs 7 iterations instead of 6, and addressing the unsafe suspension implementation in the http subsystem in code/controllers/subsystem/http.dm.

@dageavtobusnick dageavtobusnick added Testmerge PR в данный момент проходит тестирование на серверах. Testmerge Candidate PR скоро будет в тестмёрже. labels Jan 28, 2026
@littleboobs
Copy link
Contributor

@dageavtobusnick
Copy link
Collaborator Author

dageavtobusnick commented Jan 30, 2026

tgstation/tgstation#59685 (comment) ParadiseSS13/Paradise#31080 (comment)

Ты это к чему? Для констант я использую _alt варианты, В которых остается конкатинация. Все же что в рантайме заменяются на интерполяцию, которая быстрее и вызывает меньше аллокаций.

@ROdenFL
Copy link
Contributor

ROdenFL commented Jan 30, 2026

tgstation/tgstation#59685 (comment) ParadiseSS13/Paradise#31080 (comment)

Ты это к чему? Для констант я использую _alt варианты, В которых остается конкатинация. Все же что в рантайме заменяются на интерполяцию, которая быстрее и вызывает меньше аллокаций.

Мне страшно от того, что я понял, что ты сказал

@dageavtobusnick
Copy link
Collaborator Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request primarily aims at optimizing various subsystems and span macros. However, a high-severity path traversal vulnerability was identified in the new icon generation logic, as the name parameter, derived from untrusted atom names, is used without sanitization for generating filenames, which could lead to arbitrary file writes on the server. Furthermore, the changes introduce critical errors such as unsafe POW2 and POW3 macros in code/__DEFINES/math.dm due to missing parentheses, and a typo in code/game/turfs/turf.dm where wind_y_cached is incorrectly assigned wind_x during wind vector calculation. While some changes, like the security fix in globals.dm, are beneficial, these critical issues need to be addressed.

@dageavtobusnick
Copy link
Collaborator Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Upon reviewing the proposed changes, which technically look excellent, this pull request introduces several optimizations, including tick checks in subsystems and improved icon generation. However, two security vulnerabilities were identified: a potential Arbitrary File Read in the spritesheet generation system due to unsafe use of fcopy() with unvalidated icon paths, and a Log Injection vulnerability in the HTTP subsystem where untrusted response bodies are logged without sanitization. Additionally, it is recommended to use interpolation macros for all span_ calls where technically possible, as the current use of _alt macros with string concatenation may reduce optimization effectiveness.

@dageavtobusnick dageavtobusnick removed the Testmerge PR в данный момент проходит тестирование на серверах. label Feb 7, 2026
@dageavtobusnick
Copy link
Collaborator Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Аналитический отчёт от Большого Брата. Обработка запроса завершена.

Приветствую! Я проанализировал предложенные Вами изменения. С технической точки зрения, работа выглядит отлично. Вы проделали масштабную работу по оптимизации производительности и повышению стабильности кодовой базы, что заслуживает высокой оценки. Замена конкатенации строк на интерполяцию, кеширование списков, использование макросов для математических операций и рефакторинг подсистем events и http для поддержки паузы — всё это значимые улучшения. Особенно важны многочисленные исправления, связанные с утечками памяти, путём добавления и улучшения процедур Destroy().

Сводка анализа:

  • 🔴 Критических замечаний: 1
  • 🟡 Рекомендаций: 1
  • 💡 Предложений: 0

Я обнаружил одно критическое место, которое может привести к ошибке выполнения, и одну возможность для дальнейшей оптимизации. Пожалуйста, ознакомьтесь с комментариями к отдельным строкам кода для получения более подробной информации.

Спасибо за ваш вклад в проект!

@github-actions github-actions bot added the Merge Conflict PR содержит изменения, конфликтующие с master-веткой. label Feb 9, 2026
@github-actions github-actions bot removed the Merge Conflict PR содержит изменения, конфликтующие с master-веткой. label Feb 18, 2026
@github-actions github-actions bot added the Merge Conflict PR содержит изменения, конфликтующие с master-веткой. label Feb 18, 2026
@github-actions github-actions bot removed the Merge Conflict PR содержит изменения, конфликтующие с master-веткой. label Feb 18, 2026
@github-actions github-actions bot added the 🦀 Rust PR содержит изменения в файлах на языке Rust. label Feb 18, 2026
@dageavtobusnick
Copy link
Collaborator Author

!build_rust

@github-actions github-actions bot requested a review from Bizzonium as a code owner February 18, 2026 13:49
@github-actions github-actions bot added the 🛠️ Инструменты PR вносит изменения в инструменты разработки, CI/CD или конфигурации IDE. label Feb 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🦀 Rust PR содержит изменения в файлах на языке Rust. 🛠️ Инструменты PR вносит изменения в инструменты разработки, CI/CD или конфигурации IDE. Testmerge Candidate PR скоро будет в тестмёрже.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants