Skip to content

Review: Use reader/writer spin locks for ustring table. - #126

Merged
lgritz merged 1 commit into
AcademySoftwareFoundation:masterfrom
lgritz:lg-ustring
Jul 18, 2011
Merged

Review: Use reader/writer spin locks for ustring table.#126
lgritz merged 1 commit into
AcademySoftwareFoundation:masterfrom
lgritz:lg-ustring

Conversation

@lgritz

@lgritz lgritz commented Jul 18, 2011

Copy link
Copy Markdown
Collaborator

I wrote a reader/writer spin lock, used it for the ustring table, and it produces another significant improvement when there's heavy contention in ustring creation. In particular, with this patch the ustring performance continues to improve all the way up to the number of threads greater than the number of cores, whereas previously it "leveled off" earlier, with at some point additional HT cores not providing any additional benefit. Now, more cores always benefit. (For just a few cores, performance is the same as before.)

Now that this class is written, we could possibly improve performance by finding other places where a good spin rw lock is appropriate that's currently a purely exclusive lock.

Comment thread src/include/thread.h

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the compiler guaranteed to leave this alone ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

atomic int has the actual bits underneath "volatile"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be ok I guess, since m_readers is atomic and inspecting its value expands to either a function call, an intrinsic or some inline assembler depending on the compilation environment. Any code using atomics is already subtly outside the C++98 standard memory model AFAIK, so guaranteeing behaviour becomes a bit difficult. I'm pretty confident the above would be ok in the C++11 standard though...

@fpsunflower

Copy link
Copy Markdown
Contributor

LGTM

1 similar comment
@c42f

c42f commented Jul 18, 2011

Copy link
Copy Markdown
Contributor

LGTM

@lgritz
lgritz merged commit 7eee991 into AcademySoftwareFoundation:master Jul 18, 2011
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.

3 participants