Skip to content

bpo-32348: Optimize asyncio.Future schedule/add/remove callback.#4907

Merged
1st1 merged 3 commits intopython:masterfrom
1st1:fastcb
Dec 18, 2017
Merged

bpo-32348: Optimize asyncio.Future schedule/add/remove callback.#4907
1st1 merged 3 commits intopython:masterfrom
1st1:fastcb

Conversation

@1st1
Copy link
Copy Markdown
Member

@1st1 1st1 commented Dec 16, 2017

@asvetlov
Copy link
Copy Markdown
Contributor

After the change C future has callback0 member but Python version has not.
Maybe adding callback0 to py future for sake of diversity reducing makes sense?

@1st1
Copy link
Copy Markdown
Member Author

1st1 commented Dec 17, 2017

I actually want to keep Python implementation simple (for educational purposes) and different (to catch any difference in semantics).

@asvetlov
Copy link
Copy Markdown
Contributor

Got it. Ok.

Comment thread Modules/_asynciomodule.c Outdated
}
else {
assert(fut->fut_callback0 != NULL);
assert(fut->fut_callbacks == NULL);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These asserts seems too redundant.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

dropped them

Comment thread Modules/_asynciomodule.c
#define FI_FREELIST_MAXLEN 100
static futureiterobject *fi_freelist_head = NULL;
static futureiterobject *fi_freelist_tail = NULL;
static Py_ssize_t fi_freelist_len = 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I recommend LIFO than FIFO when implementing freelist.
It may be better CPU cache ratio.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

changed to LIFO

@1st1 1st1 merged commit 1b7c11f into python:master Dec 18, 2017
@1st1 1st1 deleted the fastcb branch December 18, 2017 01:19
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.

5 participants