Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

database/sql: Lock contention in func (rs *Rows) Scan #7086

Closed
yvasiyarov opened this issue Jan 9, 2014 · 5 comments
Closed

database/sql: Lock contention in func (rs *Rows) Scan #7086

yvasiyarov opened this issue Jan 9, 2014 · 5 comments
Assignees

Comments

@yvasiyarov
Copy link

@yvasiyarov yvasiyarov commented Jan 9, 2014

While doing profiling of one tool I found what function Rows.Scan() is suffering from
lock contention. The reason is what function convertAssign() use fmt.Sprintf() to
convert rows data to desired type.
So if you read a lot of data from database and use num processors > 1 you can get
result like in attached before.svg. I see there 2 problems:
1. Lock contention in function fmt.newPrinter()/free()
2. A lot of time spend inside fmt.doPrintf()

This issue is not about fmt package itself, my proposal is to use functions from strconv
package instead of calling fmt.Sprintf("%v", src). As you can see on the
second profiling, after replacing fmt.Sprintf() to
strconv.FormatBool/FormatFloat/FormatInt performance of Scanf increased about 2 times. 

I can submit my patch for this improvement if you would like to accept this issue.

Environment: 
OS: RHEL 6.5
Go version: 1.2 and development
24 CPU's

Attachments:

  1. before.svg (98258 bytes)
  2. after.svg (105460 bytes)
@dvyukov
Copy link
Member

@dvyukov dvyukov commented Jan 9, 2014

Comment 1:

On tip fmt uses sync.Pool, so hopefully it's magically resolved in Go 1.3.
Note that sync.Pool is not yet scalable, so right now performance can be the same.
@yvasiyarov
Copy link
Author

@yvasiyarov yvasiyarov commented Jan 9, 2014

Comment 2:

yes,  as far as I can see sync.Pool in development branch still use mutex for
syncronisation...  maybe better implementation is on the way.
But even if locks problem will be solved, fmt.Sprintf("%v", src) is not most efficient
way  to format bool/int/float values.
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 10, 2014

Comment 3:

Sent: https://golang.org/cl/50240044
Please review. Dmitry's improved sync.Pool would've fixed most of this, but there are
CPU reasons to avoid fmt too. And the patch above also removes a number of allocs when
using RawBytes.

Owner changed to @bradfitz.

Status changed to Started.

@dvyukov
Copy link
Member

@dvyukov dvyukov commented Jan 10, 2014

Comment 4:

> but there are CPU reasons to avoid fmt too. And the patch above also removes a number
of allocs when using RawBytes.
Sounds good.
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 10, 2014

Comment 5:

This issue was closed by revision 258ed3f.

Status changed to Fixed.

@yvasiyarov yvasiyarov added the fixed label Jan 10, 2014
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.