DynamoRIO / DynamoRIO/dynamorio

Performance: experiment with plain stores instead of xchg for hot patching

Open
#854 1 comment 0 reactions 0 assignees View on GitHub
Migrated Performance Priority-Medium Type-Feature
Dominant language
C
Stars
3.2k
Forks
629
Avg merge
2d 18h
Merged PRs (30d)
30

Description

_From [rnk@google.com](https://code.google.com/u/rnk@google.com/) on July 30, 2012 18:18:48_

Profiling shows that using xchg to atomically write jump offsets is expensive. Here's a recent profile:
[rnk@wittenberg src]$ perf report | head
# Overhead Command Shared Object Symbol
# ........ .............. ................... ...........................................................................
#
17.68% DumpRenderTree perf-14440.map [.] 0x0000000071c59213
5.14% DumpRenderTree libdynamorio.so.3.2 [.] insert_exit_stub_other_flags
4.51% DumpRenderTree libdynamorio.so.3.2 [.] hashtable_fragment_lookup.isra.31
2.27% DumpRenderTree libdynamorio.so.3.2 [.] mutex_lock
1.94% DumpRenderTree libdynamorio.so.3.2 [.] build_bb_ilist
1.92% DumpRenderTree libdynamorio.so.3.2 [.] encoding_possible_pass

The annotated asm is:
0.24 │ dc: lea 0x1(%rax),%rdx
0.16 │ movb $0xe9,(%rax)
0.16 │ sub %rdx,% r14 │ sub $0x4,%r14d
│ xchg %r14d,(%rdx)
97.40 │ add $0x5,%rax

Most of the samples here are on the add, but this is likely due to the way the samples are taken. The true cost is in the xchg, which is implicitly locked when used with a memory operand. Using a normal store on _all_ atomic writes gives a nice 5% perf improvement. I'm not sure if it's correct, however. So long as we don't cross cache lines, my understanding is that with x86 we don't get any word tearing, it's either the before or after version with respect to other observers. I don't know if the instruction fetcher is special, though.

I think we can reap the majority of the benefits by special casing insert_exit_stub_other_flags and sites like it. In this case, we're generating the full exit stub, so we don't need to use atomic instructions. We just happen to get one because we're using a utility for writing relative jump targets.

I'm not inclined to do any really large scale testing right now, so I may just go for the low hanging fruit and use normal stores just when inserting whole jump instructions.

_Original issue: http://code.google.com/p/dynamorio/issues/detail?id=854_

Contributor guide

Open the contributing guide

Research direction

Start at insert_exit_stub_other_flags and the utility used to write relative jump targets; compare the atomic xchg path with normal stores for complete jump instructions. Run the profiled workload with perf and verify that the change is safe for instruction fetching and cache-line boundaries while retaining the reported performance improvement.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, linux
Domain
performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.