Skip to content

bpo-30229: BufferedRandom.flush() avoids lseek#1386

Closed
vstinner wants to merge 1 commit into
python:masterfrom
vstinner:flush_lseek
Closed

bpo-30229: BufferedRandom.flush() avoids lseek#1386
vstinner wants to merge 1 commit into
python:masterfrom
vstinner:flush_lseek

Conversation

@vstinner

@vstinner vstinner commented May 2, 2017

Copy link
Copy Markdown
Member

BufferedRandom flush() and close() don't call lseek() syscall anymore if the read and write buffers are empty.

BufferedRandom flush() and close() don't call lseek() syscall anymore
if the read and write buffers are empty.
@vstinner vstinner added the performance Performance or resource usage label May 2, 2017
@mention-bot

Copy link
Copy Markdown

@Haypo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benjaminp, @serhiy-storchaka and @shibturn to be potential reviewers.

@vstinner

vstinner commented May 3, 2017

Copy link
Copy Markdown
Member Author

@pitrou : would you mind to review this change? I guess that you wrote that code ;-)

@pitrou

pitrou commented May 3, 2017

Copy link
Copy Markdown
Member

I'm not sure it's a good idea to change this code if there is not a pressing need. Which performance difference do you see?

@vstinner

vstinner commented May 3, 2017 via email

Copy link
Copy Markdown
Member Author

@vstinner

Copy link
Copy Markdown
Member Author

I'm not sure about this change and @pitrou doesn't want to take the any risk, so I reject this change.

I will now focus on the PR #1385 which removes another syscall and IMHO is simpler.

@vstinner vstinner closed this Jun 14, 2017
@vstinner
vstinner deleted the flush_lseek branch June 14, 2017 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance or resource usage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants