Skip to content
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

fix(core): import time latency by lazily loading high level modules #301

Merged
merged 17 commits into from Mar 3, 2021

Conversation

heitorlessa
Copy link
Contributor

@heitorlessa heitorlessa commented Mar 1, 2021

Issue #, if available: #294

Description of changes:

Checklist

  • Meet tenets criteria
  • Update tests
  • Update docs
  • PR title follows conventional commit semantics
  • Create generic types for Tracer provider
  • Create generic types for Tracer subsegments
  • Convert EMF JSON Schema into native object to reduce I/O
  • Lazily load aws_xray_sdk
  • Lazily load aws_xray_sdk.core
  • Lazily load fastjsonschema

Changes table

Scenario Result Change
Logger import only 0.33s user 0.15s system 82% cpu 0.582 total No change
Logger import only 0.07s user 0.02s system 97% cpu 0.102 total Tracer lazily loaded
Tracer imported but not initialized 0.08s user 0.02s system 84% cpu 0.115 total Tracer lazily loaded
Tracer imported and initialized 0.32s user 0.11s system 92% cpu 0.469 total Tracer lazily loaded
Logger import only negligible difference Metrics EMF JSON Schema as dict

Notes

  • LazyLoader adds roughly 10ms due to class instantiation and globals manipulation
  • Hard lesson: Type annotations load lazily loaded library ahead of time diminishing its effectiveness

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@heitorlessa heitorlessa marked this pull request as draft March 1, 2021 08:59
@heitorlessa heitorlessa marked this pull request as ready for review March 1, 2021 10:30
@heitorlessa heitorlessa changed the title fix: import time latency by lazily loading high level modules WIP fix: import time latency by lazily loading high level modules Mar 1, 2021
@heitorlessa heitorlessa changed the title WIP fix: import time latency by lazily loading high level modules [WIP] fix: import time latency by lazily loading high level modules Mar 1, 2021
@heitorlessa heitorlessa changed the title [WIP] fix: import time latency by lazily loading high level modules fix: import time latency by lazily loading high level modules Mar 1, 2021
@michaelbrewer
Copy link
Contributor

@heitorlessa nice, definitely handy for lambdas that just want structured logging or want to toggle tracing only for dev and test and disable on production.

@heitorlessa
Copy link
Contributor Author

heitorlessa commented Mar 2, 2021

BEFORE

image

AFTER

New flamegraph with these patches - Tracer no longer appears and removes ~400ms for all imports and customers once merged

image

@heitorlessa
Copy link
Contributor Author

Done - @nmoutschen, do you have the bandwidth to review?

Copy link
Contributor

@nmoutschen nmoutschen left a comment

Choose a reason for hiding this comment

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

I have a few questions/remarks before I can approve. We also need to include the proper licensing for the class taken from Terraform (Apache-2.0 license).

aws_lambda_powertools/tracing/tracer.py Show resolved Hide resolved
aws_lambda_powertools/tracing/tracer.py Show resolved Hide resolved
tests/performance/test_high_level_imports.py Outdated Show resolved Hide resolved
tests/performance/test_high_level_imports.py Outdated Show resolved Hide resolved
Copy link
Contributor

@nmoutschen nmoutschen left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-io
Copy link

codecov-io commented Mar 3, 2021

Codecov Report

Merging #301 (01445a3) into develop (fe53a2e) will decrease coverage by 0.05%.
The diff coverage is 96.96%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #301      +/-   ##
===========================================
- Coverage    99.71%   99.65%   -0.06%     
===========================================
  Files           87       90       +3     
  Lines         3150     3206      +56     
  Branches       149      150       +1     
===========================================
+ Hits          3141     3195      +54     
- Misses           5        7       +2     
  Partials         4        4              
Impacted Files Coverage Δ
...mbda_powertools/utilities/parser/envelopes/base.py 100.00% <ø> (ø)
aws_lambda_powertools/shared/lazy_import.py 89.47% <89.47%> (ø)
aws_lambda_powertools/metrics/base.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/schema.py 100.00% <100.00%> (ø)
aws_lambda_powertools/shared/constants.py 100.00% <100.00%> (ø)
aws_lambda_powertools/tracing/base.py 100.00% <100.00%> (ø)
aws_lambda_powertools/tracing/tracer.py 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe53a2e...01445a3. Read the comment docs.

@heitorlessa heitorlessa merged commit 11ebcf9 into aws-powertools:develop Mar 3, 2021
@heitorlessa heitorlessa deleted the fix/import-latency branch March 3, 2021 09:22
@heitorlessa heitorlessa changed the title fix: import time latency by lazily loading high level modules fix(core): import time latency by lazily loading high level modules Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants