Skip to content

Commit 9159b81

Browse files
authored
chore(iast): optimize iast by migrating should_iast_patch function to c (#12774)
**TL;DR: Before this PR, IAST startup time was x5,16 lower than an application without IAST startup time After this PR, IAST startup time was x2,79** ## Description In this PR, we aim to improve the startup times of an application when IAST (Interactive Application Security Testing) is enabled. The following improvements have been made: ### Performance improvements - These changes aim to streamline the codebase, improve the functionality of the IAST module, and remove outdated or redundant code.When reading the source code of a module for transformation with AST, it is now done in binary mode, which is **30% faste** as shown in this gist: https://gist.github.com/avara1986/8e31a23978b5eecd9dc8f2f3135060b0. The execution times are: - `open("r")`: 0.9285 secs - `open("rb")`: 0.6362 secs - The function `should_iast_patch` has been migrated to C. By comparing the original function with the new one using the following `timeit` command, is **40% faster**. ``` # refactor python -m timeit -n 5000000 -r 5 -u usec -s "from ddtrace.appsec._iast._ast import iastpatch""iastpatch.should_iast_patch('mimodulo.submodulo')" # main python -m timeit -n 5000000 -r 5 -u usec -s "from ddtrace.appsec._iast._ast.ast_patching import _should_iast_patch" "_should_iast_patch('package.mypackage')" ``` - I wanted to migrate `exec` to `PyEval_EvalCode` but the results are very revealing, exec is approximately **17% faster**. The results of this gist https://gist.github.com/avara1986/ae2a65dca1b90afa762af67db680bc2b - `exec`: mean time of 0.083023 seconds - `PyEval_EvalCode`: mean time of 0.099550 seconds By combining all these changes, the microbenchmarks for startup times show that the baseline takes 2.46 seconds, while this branch takes 1.24 seconds, making it **1.99x faster**. ### Performance analysis The `_should_iast_patch` logs are under asm_config._iast_debug because Debug mode has a significant performance impact, making the function between 1.14x and 3.08x slower ``` Benchmark Results -------------------------------------------------------------------------------- Module Name Debug Time (s) No Debug Time (s) Ratio -------------------------------------------------------------------------------- os 0.017021 0.005522 3.08x requests 0.023237 0.020418 1.14x ddtrace.appsec._iast 0.024511 0.012280 2.00x non.existent.module 0.023403 0.011170 2.10x ``` Branch deployed in rel-env and no leaks found: ![image](https://github.com/user-attachments/assets/faaa6ead-6c48-4f05-944b-5d29ed338d9a) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
1 parent 36dcd9f commit 9159b81

24 files changed

+1210
-2230
lines changed

ddtrace/appsec/_iast/__init__.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,6 @@ def enable_iast_propagation():
9292
global _iast_propagation_enabled
9393
if _iast_propagation_enabled:
9494
return
95-
# We need to preload the package_distributions because if we call importlib_metadata inside a module hook
96-
# it raises a circular-import
97-
from ddtrace.internal.packages import get_package_distributions
98-
99-
get_package_distributions()
10095

10196
log.debug("iast::instrumentation::starting IAST")
10297
ModuleWatchdog.register_pre_exec_module_hook(_should_iast_patch, _exec_iast_patched_module)
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
cmake_minimum_required(VERSION 3.10)
2+
project(iastpatch C)
3+
4+
find_package(Python3 REQUIRED COMPONENTS Development)
5+
6+
add_library(iastpatch MODULE iastpatch.c)
7+
8+
target_include_directories(iastpatch PRIVATE ${Python3_INCLUDE_DIRS})
9+
10+
target_link_libraries(iastpatch PRIVATE ${Python3_LIBRARIES})
11+
12+
set_target_properties(iastpatch PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}")

0 commit comments

Comments
 (0)