Skip to content

Make airflow --help run five times quicker#12836

Merged
ashb merged 1 commit intoapache:masterfrom
astronomer:faster-airflow-help
Dec 5, 2020
Merged

Make airflow --help run five times quicker#12836
ashb merged 1 commit intoapache:masterfrom
astronomer:faster-airflow-help

Conversation

@ashb
Copy link
Member

@ashb ashb commented Dec 5, 2020

Importing anything from airflow.models pulls in a lot of airflow, so
delay imports until the functions are called, or make use of the
TYPE_CHECKING to not actually import at runtime.

Before: mean 2.58s (with a lot of variance)

airflow ❯ for i in 1 2 3; do time airflow --help >/dev/null; done
airflow --help > /dev/null  2.00s user 1.39s system 176% cpu 1.928 total
airflow --help > /dev/null  2.84s user 1.43s system 151% cpu 2.817 total
airflow --help > /dev/null  3.00s user 1.37s system 145% cpu 3.009 total

After: 0.526s

airflow --help > /dev/null  0.39s user 0.04s system 99% cpu 0.435 total
airflow --help > /dev/null  0.40s user 0.05s system 99% cpu 0.446 total
airflow --help > /dev/null  0.64s user 0.05s system 99% cpu 0.698 total

This also has an advantage in development where a syntax error doesn't
fail with a slightly confusing error message about "unable to configure
logger 'task'".


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@ashb ashb added the full tests needed We need to run full set of tests for this PR to merge label Dec 5, 2020
@ashb
Copy link
Member Author

ashb commented Dec 5, 2020

Discovered with

python -X importtime -m airflow version 2>import-trace.log

And then tuna import-trace.log

Before
image

After

image

@ryw
Copy link
Member

ryw commented Dec 5, 2020

watched you code this live :)

@github-actions
Copy link

github-actions bot commented Dec 5, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

github-actions bot commented Dec 5, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

github-actions bot commented Dec 5, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

Importing anything from airflow.models pulls in _a lot_ of airflow, so
delay imports until the functions are called, or make use of the
`TYPE_CHECKING` to not actually import at runtime.

**Before**: mean 2.58s (with a lot of variance)
```
airflow ❯ for i in 1 2 3; do time airflow --help >/dev/null; done
airflow --help > /dev/null  2.00s user 1.39s system 176% cpu 1.928 total
airflow --help > /dev/null  2.84s user 1.43s system 151% cpu 2.817 total
airflow --help > /dev/null  3.00s user 1.37s system 145% cpu 3.009 total

```

**After**: 0.526s

```
airflow --help > /dev/null  0.39s user 0.04s system 99% cpu 0.435 total
airflow --help > /dev/null  0.40s user 0.05s system 99% cpu 0.446 total
airflow --help > /dev/null  0.64s user 0.05s system 99% cpu 0.698 total
```

This also has an advantage in development where a syntax error doesn't
fail with a slightly confusing error message about "unable to configure
logger 'task'".
@ashb ashb force-pushed the faster-airflow-help branch from 0313409 to a996aad Compare December 5, 2020 18:39
@potiuk
Copy link
Member

potiuk commented Dec 5, 2020

I'd prefer to solve it differently, but pragmatically, this is easiest way now.

@ashb
Copy link
Member Author

ashb commented Dec 5, 2020

I'd prefer to solve it differently, but pragmatically, this is easiest way now.

I would also like that importing modules had less "side-effects", but...

I think we need both ways -- because fundamentally, even if importing from models imported less code, there's still the fact that in some cases we are only importing it for a type annotation, which is just wasteful at runtime

@ashb ashb merged commit 37b2679 into apache:master Dec 5, 2020
@ashb ashb deleted the faster-airflow-help branch December 5, 2020 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:logging full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants