Skip to content

Convert dag.tasks list to dict to avoid slowdown#1368

Closed
jlowin wants to merge 1 commit intoapache:masterfrom
jlowin:task-dict
Closed

Convert dag.tasks list to dict to avoid slowdown#1368
jlowin wants to merge 1 commit intoapache:masterfrom
jlowin:task-dict

Conversation

@jlowin
Copy link
Member

@jlowin jlowin commented Apr 13, 2016

After discussion with @mistercrunch, looks like #1213 introduced some slowdowns. I believe it's related to increased use of dag.get_task() which iterates over all tasks in dag.tasks. For example: https://github.com/airbnb/airflow/pull/1213/files#diff-a32a363fa616685db3bfefba947535b2R1767

The solution is to replace the list of tasks with a {task_id: task} dict. Adding a tasks property that returns a list of the dict's values means all other uses of dag.tasks should work.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling fd605d4 on jlowin:task-dict into 9689159 on airbnb:master.

@jlowin jlowin changed the title WIP: Convert task list to dict Convert dag.tasks list to dict to avoid slowdown Apr 13, 2016
@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling f7f2084 on jlowin:task-dict into 9689159 on airbnb:master.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling f7f2084 on jlowin:task-dict into 9689159 on airbnb:master.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.04% when pulling e468e70 on jlowin:task-dict into 9689159 on airbnb:master.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.04% when pulling 81bf781 on jlowin:task-dict into 9689159 on airbnb:master.

@mistercrunch
Copy link
Member

Nice, I'm glad to find this, I'll just take it on from here

@r39132
Copy link
Contributor

r39132 commented Apr 13, 2016

+1

@jlowin jlowin deleted the task-dict branch April 13, 2016 23:27
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.

4 participants