Skip to content

Conversation

@purple4reina
Copy link
Contributor

@purple4reina purple4reina commented Mar 20, 2025

This PR lazy loads a couple of packages so they are only imported when needed.

Checklist

  • 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
  • The change includes or references documentation updates if necessary
  • Backport labels are set (if applicable)

Reviewer Checklist

  • 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 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

@github-actions
Copy link
Contributor

CODEOWNERS have been resolved as:

ddtrace/_trace/trace_handlers.py                                        @DataDog/apm-sdk-api-python
tests/internal/test_serverless.py                                       @DataDog/apm-core-python @DataDog/apm-serverless

@github-actions
Copy link
Contributor

github-actions bot commented Mar 20, 2025

Bootstrap import analysis

Comparison of import times between this PR and main.

Summary

The average import time in this PR is: 226 ± 2 ms.

The average import time in main is: 237 ± 2 ms.

The import time difference between this PR and main is: -10.74 ± 0.07 ms.

Import time breakdown

The following import paths have disappeared:

ddtrace.auto 9.149 ms (4.04%)
ddtrace.bootstrap.sitecustomize 9.149 ms (4.04%)
ddtrace._trace.trace_handlers 9.149 ms (4.04%)
ddtrace._trace.utils_botocore.span_tags 6.463 ms (2.85%)
ddtrace._trace.utils_botocore.aws_payload_tagging 5.902 ms (2.61%)
ddtrace.vendor.jsonpath_ng 3.999 ms (1.77%)
ddtrace.vendor.jsonpath_ng.jsonpath 2.315 ms (1.02%)
ddtrace.vendor.jsonpath_ng.lexer 1.643 ms (0.73%)
ddtrace.vendor.ply.lex 0.999 ms (0.44%)
ddtrace.vendor.ply 0.247 ms (0.11%)
ddtrace.vendor.jsonpath_ng.exceptions 0.296 ms (0.13%)
ddtrace.vendor.jsonpath_ng.parser 1.412 ms (0.62%)
ddtrace.vendor.ply.yacc 1.035 ms (0.46%)
decimal 1.589 ms (0.70%)
_decimal 1.358 ms (0.60%)
numbers 0.565 ms (0.25%)
ddtrace.ext.aws 0.271 ms (0.12%)
ddtrace._trace.utils_botocore.span_pointers 2.686 ms (1.19%)
ddtrace._trace.utils_botocore.span_pointers.dynamodb 1.449 ms (0.64%)
ddtrace._trace.utils_botocore.span_pointers.telemetry 0.264 ms (0.12%)
ddtrace._trace.utils_botocore.span_pointers.s3 0.628 ms (0.28%)
ddtrace._trace.utils_botocore 0.249 ms (0.11%)

The following import paths have grown:

ddtrace.auto 0.171 ms (0.08%)
ddtrace.bootstrap.sitecustomize 0.171 ms (0.08%)
ddtrace._trace.trace_handlers 0.078 ms (0.03%)
ddtrace.contrib.trace_utils 0.078 ms (0.03%)
ddtrace.contrib.internal.trace_utils 0.078 ms (0.03%)
ddtrace.ext.user 0.078 ms (0.03%)
ddtrace.bootstrap.preload 0.020 ms (0.01%)
ddtrace.internal.products 0.020 ms (0.01%)
ddtrace.settings.dynamic_instrumentation 0.020 ms (0.01%)

The following import paths have shrunk:

ddtrace.auto 2.094 ms (0.92%)
ddtrace.bootstrap.sitecustomize 1.435 ms (0.63%)
ddtrace.bootstrap.preload 1.284 ms (0.57%)
ddtrace.internal.products 1.284 ms (0.57%)
ddtrace.internal.remoteconfig.client 0.641 ms (0.28%)
ddtrace._trace.trace_handlers 0.076 ms (0.03%)
ddtrace.contrib.internal.botocore.constants 0.076 ms (0.03%)
ddtrace.contrib.internal.botocore 0.076 ms (0.03%)
ddtrace.appsec._common_module_patches 0.075 ms (0.03%)
ddtrace.appsec._asm_request_context 0.075 ms (0.03%)
ddtrace 0.659 ms (0.29%)

@purple4reina
Copy link
Contributor Author

Reduces cold start time in aws lambda by about 2.5%

Screenshot 2025-03-20 at 2 16 34 PM

@pr-commenter
Copy link

pr-commenter bot commented Mar 20, 2025

Benchmarks

Benchmark execution time: 2025-03-24 21:37:52

Comparing candidate commit 5a900df in PR branch rey.abolofia/lazy-pointers with baseline commit 08ee25b in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 478 metrics, 2 unstable metrics.

@purple4reina purple4reina added the changelog/no-changelog A changelog entry is not required for this PR. label Mar 24, 2025
@purple4reina purple4reina marked this pull request as ready for review March 24, 2025 20:51
@purple4reina purple4reina requested review from a team as code owners March 24, 2025 20:51
@purple4reina purple4reina changed the title perf(serverless): lazy import botocore code. perf(serverless): lazy import botocore code Mar 24, 2025
@purple4reina purple4reina enabled auto-merge (squash) March 25, 2025 16:28
Copy link
Member

@brettlangdon brettlangdon left a comment

Choose a reason for hiding this comment

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

I don't love adding the imports into the event handlers, since if they are called at a high rate, then might have a perf impact. I don't think we have benchmarks to validate this though.

Given the other comments about potentially refactoring this to d patch on import/remove integration specific handlers from always being loaded, I think that would alleviate any initial feelings I have over perf.

That all being said, think we can handle things as follow-ups.

@purple4reina purple4reina merged commit 1e04c2e into main Mar 25, 2025
791 of 792 checks passed
@purple4reina purple4reina deleted the rey.abolofia/lazy-pointers branch March 25, 2025 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/no-changelog A changelog entry is not required for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants