Skip to content

Conversation

@shinrich
Copy link
Member

@shinrich shinrich commented Feb 1, 2019

Going through the great list of local fixes while getting ready for the 8.x sync. We had been seeing performance hot spots with these unnecessary read_avail calls.

@shinrich shinrich added this to the 9.0.0 milestone Feb 1, 2019
@shinrich shinrich self-assigned this Feb 1, 2019
@SolidWallOfCode
Copy link
Member

Yes, for reasons we haven't discovered, the block list can become quite long and read_avail walks the entire list every time. In contrast, read_avail_more_than(0) will just look at the first block.

@masaori335
Copy link
Contributor

masaori335 commented Feb 4, 2019

I agree with that calling read_avail() should be avoided as much as possible. We observed that read_avail() made spin with long IOBufferBlock in other area in the past.

Looks this changes is doing logically same things without read_avail(). I bit worried about replacing memcpy() & consume() to read(), but it looks OK so far.

@SolidWallOfCode
Copy link
Member

Actually, @masaori335, I think that's a bug. I want to talk with Susan before we commit this. I don't think read does the same thing as consume in this context.

@SolidWallOfCode
Copy link
Member

Spoke with Susan, I misunderstood the IOBufferReader API. The code looks correct.

@shinrich shinrich merged commit fc4975d into apache:master Feb 4, 2019
@bryancall bryancall modified the milestones: 9.0.0, 8.1.0 Mar 27, 2019
@bryancall
Copy link
Contributor

Cherry picked to 8.1.0

@zwoop zwoop modified the milestones: 8.1.0, 8.1.0-nogo Mar 18, 2020
@zwoop zwoop modified the milestones: 8.1.0-nogo, 8.1.0 Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants