This option turns on a frame counter that is shown in the top left corner via a
QR code. It was designed to be used in video recordings of B2G phones.
It no longer seems useful, so this patch removes it.
* * *
Bug 1357298 - Remove all traces of frame numbers and power from the profiler output. r=mstange.
--HG--
extra : rebase_source : 0ce87963ce375df64bb8d80ef2b5d40ea507bc7c
This fixes a JS exception that gets thrown when one tries to capture a profile
in this case.
--HG--
extra : rebase_source : 46f6eeed3c17086b0b6c35b26f3c9e4841dd6cff
clang's -Wcomma warning warns about suspicious use of the comma operator such as between two statements.
tools/profiler/lul/LulDwarf.cpp:604:15: warning: possible misuse of comma operator here [-Wcomma]
MozReview-Commit-ID: 6ZP79hgtrAD
--HG--
extra : rebase_source : 77028600c713aa3235c3729a5db7be0290df57e4
extra : source : e4536bbeb28050b38979a05b379f13eb4a12beee
The patch adds a missing |delete|. (This leak only occurred when the
"mainthreadio" feature was enabled, which is not the default.)
The patch also makes the unregistering of the interpose observer conditional on
there being one in the first place, avoiding a harmless but useless
unregistering of |nullptr|.
--HG--
extra : rebase_source : 7cc3679192e3effa8d86edad5374643d2e2b8948
For reasons related to the architecture of the Gecko Profiler in previous years,
which are no longer relevant, LUL will only unwind through the first 32KB of
stack. This is mostly harmless, since most stacks are smaller than 4KB, per
measurements today, but occasionally they go above 32KB, causing unwinding to
stop prematurely.
This patch changes the max size to 160KB, and documents the rationale for
copying the stack and unwinding, rather than unwinding in place. 160KB is big
enough for all stacks observed in several minutes of profiling all threads at
1KHz.
--HG--
extra : rebase_source : a1d5526aff50345be8b965c2b6b01c66b40fd0d8
Everything depending on the widget being gonk can go away, as well as
everything depending on MOZ_AUDIO_CHANNEL_MANAGER, which was only
defined on gonk builds under b2g/ (which goes away in bug 1357326).
--HG--
extra : rebase_source : 9f0aeeb7eea8417fa4e06d662d566d67ecaf2a24
Bump the version number to match what is currently published.
MozReview-Commit-ID: 8r8otQQBqBo
--HG--
extra : rebase_source : 1b7daac6b2852117ae08927fd5a09b2a7650d683
This function can run off the main thread when 'layers.frame-counter' is
enabled.
--HG--
extra : rebase_source : f3db0fd01c636d5af97109761bb0b57b57c79293
This also renames HasProfile() to IsBeingProfiled().
MozReview-Commit-ID: 70RGHNbyZG3
--HG--
extra : rebase_source : 64f1df6985f41ae52d2385edcfd7822d16fd1e00
This also renames HasProfile() to IsBeingProfiled().
MozReview-Commit-ID: 70RGHNbyZG3
--HG--
extra : rebase_source : fbe6faf0ed9ee7273e77f1f81b79915800772212
For reasons which are unclear, but possibly due to lack of any known use cases
when the code was written, LUL on i686/x86_64-linux only accepts CFA (canonical
frame address) expressions of the form SP+offset or FP+offset. However, on
Fedora 25 x86_64 and Ubuntu 16.10 x86_64, at least one address range per object
uses a Dwarf expression for the CFA, for example:
00000018 000000000024 0000001c FDE cie=00000000 pc=0000000031e0..0000000031f0
DW_CFA_def_cfa_offset: 16
DW_CFA_advance_loc: 6 to 00000000000031e6
DW_CFA_def_cfa_offset: 24
DW_CFA_advance_loc: 10 to 00000000000031f0
DW_CFA_def_cfa_expression(
DW_OP_breg7 (rsp): 8; DW_OP_breg16 (rip): 0; DW_OP_lit15; DW_OP_and;
DW_OP_lit11; DW_OP_ge; DW_OP_lit3; DW_OP_shl; DW_OP_plus)
producing the following complaint from LUL:
can't summarise: SVMA=0x31f0: rule for DW_REG_CFA: invalid |how|, expr=LExpr(PFXEXPR,0,0)
Given that LUL is capable of handling such a CFA expression, it seems artificial
to stop it doing so. This patch changes Summariser::Rule() so as to allow such
expressions.
PseudoStack requires that startJSSampling() and stopJSSampling() calls be
interleaved. But currently the conditions guarding those calls don't match:
startJSSampling() is guarded by ShouldProfileThread(), and stopJSSampling() is
guarded by HasProfile().
It's possible for HasProfile() to be true when ShouldProfileThread() is not
true -- e.g. profile many threads, then restart and profile fewer threads, and
we end up with live threads that have a profile but aren't being profiled right
now -- which leads to assertion failures in stopJSSampling().
This patch makes the stopJSSampling() condition use ShouldProfileThread(), just
like the startJSSampling() condition, which fixes the assertion failure.
--HG--
extra : rebase_source : e9931928c8ac1301f5018f9da319bc478722b98e
LUL doesn't read CFI from the main executable on x86_64-linux, and possibly
other Linux variants, because SharedLibraryInfo::GetInfoForSelf() doesn't
produce a name for the main executable object, even though it does notice the
mapping.
This causes noticeable unwind breakage because the main executable on Linux
contains various wrapper functions pertaining to memory allocation and locking,
such as
moz_xmalloc, moz_xcalloc, moz_xrealloc
mozilla::detail::MutexImpl::lock, mozilla::detail::MutexImpl::unlock
and is generally observable on x86_64-Linux as unwinding failures out of
functions with addresses around 0x40xxxx, since that's the traditional load
address for the main executable.
This patch modifies the Linux implementation of GetInfoForSelf() so as to
harvest the main executable's name from /proc/self/maps. This is then added
into the information acquired from dl_iterate_phdr. As a result
GetInfoForSelf() does correctly report the executable name, so LUL reads Dwarf
unwind info from it, and the abovementioned unwinding failures disappear.
--HG--
extra : rebase_source : 267c6d7c3967a4d29f8ff0b4a91d339a6625085d
Pick up autofix improvements in 3.19.0. Upgrade eslint-plugin-react. Also fix issues with the package.json file and the ESLint node_modules upload script.
MozReview-Commit-ID: IDZ1n4qTTuv
--HG--
extra : rebase_source : aa97cd6f314ce10d16d12446e50a27d6f994a9f1
shared-linux-libraries.cc is a maze of ifdefs which is hard to navigate, hard to
reason about and gets in the way of making a proper fix for bug 1354546. This
bug is for cleanup only. It should not change any functionality.
The following changes are made:
* adds emacs/vi tab-width lines
* removes the ARRAY_SIZE macro as it appears to be unused
* documents the 3 different configurations, splits #includes accordingly
* comments SharedLibraryInfo::GetInfoForSelf accordingly
* wraps some long lines
* documents in which cases dl_iterate_phdr is used and in which cases
/proc/<pid>/maps is used
* Puts /proc/<pid>/maps reading in its own scope
* Makes the LOG messages on failure clearer
Currently, ThreadInfos for live and dead threads are stored in a single vector.
This patch separates them into two separate vectors.
This ensures that the two kinds of ThreadInfos can't be mixed up. It also means
ThreadInfo::mPendingDelete can be removed.
Currently, when the profiler is active we hold onto the ThreadInfo of all
threads that die. Then when capturing a profile we ignore all threads that
aren't being profiled.
This patch changes things so we only hold onto the ThreadInfos of threads that
die if they are being profiled. In effect it removes state 3 from the following
list of possible ThreadInfo states:
1. !PendingDelete + !HasProfile
2. !PendingDelete + HasProfile
3. PendingDelete + !HasProfile (no longer used)
4. PendingDelete + HasProfile
Now that ThreadResponsiveness is only used on the main thread, we can refactor
ThreadInfo a bit. This patch does the following.
- Removes ThreadInfo::mThread, which is unused.
- Changes ThreadInfo::mRespInfo to a Maybe<>, and moves the is-main-thread
checking outside of ThreadInfo and ThreadResponsiveness.
- Renames {ThreadInfo,TickSample}::mRespInfo as mResponsiveness, to better
match the class name.
The state management is better done within nsProfiler::GetProfileDataAsync()
and nsProfiler::DumpProfileToFileAsync(). (The latter function is new in this
patch.)
This fixes a deadlock.
Other notes:
- The patch moves ProfileGatherer from ProfilerState to nsProfiler. This is
nice because the former is shared between threads but the latter is main
thread only. (This is how the deadlock is avoided.)
- ProfilerStateMutex and PSLockRef are no longer required in platform.h. Those
types and variables are now only used in platform.cpp and platform-*.cpp.
- ProfilerGatherer now calls profiler_get_profile() instead of ToJSON(). Which
means that ToJSON() now has a single caller, so the patch inlines it at the
callsite and removes it.
- profiler_save_profile_to_file_async() dispatched a Runnable to the main
thread. But this wasn't necessary, because it always ran on the main thread
itself. So the new function nsProfiler::DumpProfileToFileAsync() doesn't do
that.
- profiler_will_gather_OOP_profile(), profiler_gathered_OOP_profile(), and
profiler_OOP_exit_profile() are all moved into nsProfiler as well. This
removes the need for the horrible fake lock in
profiler_will_gather_OOP_profile(), hooray!
The conversion to a JSObject is better done within
nsProfiler::GetProfileData().
--HG--
extra : rebase_source : 4a0ba97d99681fca96f2d26b609bafe188095787
This reduces the amount of places where we need to specify the mozilla/frame-script environment. It does have
the side effect of allowing those globals in the whole file, but that is what specifying the environment would
do, and this is also for mochitest test files only.
MozReview-Commit-ID: 1LLFbn6fFJR
--HG--
extra : rebase_source : 82a6934d90bbbbd25f91b7b06bf4f9354e38865a
This retains the advantage of running only once per process, while
avoiding the per-process overhead of a jsm.
MozReview-Commit-ID: 1N53MvRwUpg
--HG--
rename : browser/modules/ContentObservers.jsm => browser/modules/ContentObservers.js
extra : rebase_source : 6a502cff26fcb55526f97385274bbae871f5cc6c
SetSampleContext() sets the TickSample's register fields, and the two callers
of SetSampleContext() set the TickSample's mContext.
This patch changes SetSampleContext() so it sets all the fields in one place.
It also renames SetSampleContext() as FillInSample(), because it sets more than
just the context.
--HG--
extra : rebase_source : 9b9f749fe3de687a7fd32f5c38e2321c2abebfdc
Currently each live thread has a PseudoStack that is owned by tlsPseudoStack,
and a ThreadInfo that has a non-owning pointer to the same PseudoStack.
Then, if the profile is active when the thread dies, ownership of the
PseudoStack is transferred to the ThreadInfo.
This patch simplifies the ownership rules. Every ThreadInfo now always owns its
PseudoStack and is responsible for destroying it. tlsPseudoStack is a
non-owning pointer, and so must be cleared when a PseudoStack is destroyed.
This simplifies the code in a few places.
--HG--
extra : rebase_source : 1012b6590380091d60eff98b4e0c5b1ba946cc7e
The patch also adds a MOZ_RELEASE_ASSERT in profiler_unregister_thread() for
the case where the ThreadInfo isn't found, which is informative.
--HG--
extra : rebase_source : 11a86914db235e4a60955ff1c9b77d46109af548
This avoids the need for the fake ThreadInfo in profiler_get_backtrace(). It
requires adding a few extra fields to TickSample.
--HG--
extra : rebase_source : c28e5493edc7db96a7160e78b297ae09dc05ca7c
This patch does the following.
- Splits TickSample's constructor in two, one for the periodic sample case, and
one for the synchronous sample case, and initializes more stuff in them. (The
two constructors aren't that different right now, but they will become more
different when I remove TickSample::mThreadInfo.)
- Makes all the constructor-filled fields in TickSample |const|.
- Reorders the fields so that the constructor-filled ones are before the ones
that get filled in later.
- Omits mContext on Mac via conditional compilation, to make the omission
clearer.
--HG--
extra : rebase_source : f3e392c4cf777df5b9f39577af82615890137018
LastSample only makes sense for periodic samples, which are written to the
global ProfileBuffer. It doesn't make sense for synchronous samples which are
written to their own unshared buffer. At the moment it doesn't hurt to use
them in this nonsensical way, but the ThreadInfo profiler_get_backtrace()
will be removed soon, and we won't even have a LastSample to use nonsensically.
So this patch makes the LastSample argument to addTagThreadId() optional. Which
means we have to pass in a ThreadId, so there's no longer much point
duplicating the ThreadId in LastSample, so the patch removes that field too.
This avoids the possibility of the duplicate ThreadId failing to match, which
is nice.
--HG--
extra : rebase_source : dad76ff8b33663398e6f45f85da500b0fd7a598f
Editors generally look for configurations at the top level of a project. For ESLint, they also look for the specific binary in node_modules before defaulting to the system binary. Whilst you can override the location, generally it doesn't work well when switching between projects.
The custom in-tree libraries make setup of a system ESLint more difficult as well.
Therefore to make it simple for developers to pick up the ESLint integrations with Editors, by moving the package.json and associated node_modules to the top-level directory.
MozReview-Commit-ID: 1pQpd7hTQ61
--HG--
rename : tools/lint/eslint/npm-shrinkwrap.json => npm-shrinkwrap.json
rename : tools/lint/eslint/package.json => package.json
extra : rebase_source : 9d69d791f86b5c55b1fcd5f6449f0ab84e56b05c
At this point the only things in the ThreadInfo it uses are the thread name and
id, which are easy to store instead. This gets a step closer to avoiding the
use of ThreadInfo in profiler_get_backtrace().
--HG--
extra : rebase_source : f4feb08ec9fe7880ee43f784c6878c1c04fd3294
StreamSamplesAndMarkers() is the only ThreadInfo method called on
ProfilerBacktrace::mThreadInfo. Furthermore, it doesn't use all that much stuff
from ThreadInfo, and what stuff it does use we can instead pass in as
arguments.
This patch moves StreamSamplesAndMarkers() out of the class. It's a little
ugly, but a necessary precursor for removing ProfilerBacktrace::mThreadInfo and
all the subsequent improvements.
--HG--
extra : rebase_source : 417bda4f29a27c525f7240d3427494dd86b9a868
This required a tweak to DoNativeBacktrace() to work around an ASAN false
positive.
--HG--
extra : rebase_source : 2e21ae4c132db812150f42c26aa708aefce311be
When the profiler is running in privacy mode, we don't want to include dynamic
strings from PROFILER_LABEL_DYNAMIC to end up in the profile.
Rather than checking this every time we enter a scope marked with
PROFILER_LABEL_DYNAMIC, with this patch we will push the dynamic string into
the pseudo stack entry regardless, and then check the privacy mode during
sampling and ignore the dynamic string as necessary.
This way we can avoid taking the profiler state lock in PROFILER_LABEL_DYNAMIC
and also save a branch.
MozReview-Commit-ID: 5dXrtMuFJ5r
--HG--
extra : rebase_source : 1c2057e7ced332d9001137b5b280feab77a712e5
The checking on the return of value of fetch/locate is not working as intended.
The code should be early returning, but the return value of fetch/locate is 1 in
failure cases, which will not trigger the early return. The return value has
been altered to None, which will trigger the early return.
MozReview-Commit-ID: FDCW6rbZI6T
--HG--
extra : rebase_source : 8b087174db268d7d6a0bb62e6537f82a3b15b28d
"Metadata" regularly confuses me, because it suggests something complicated
rather than a simple enum.
This change also has the benefit of removing inconsistent capitalization
("Metadata" vs. "MetaData").
--HG--
extra : rebase_source : b651e124142c8d93139d22dae1c993c899be4d7a
This patch:
- Removes TRACING_EVENT_BACKTRACE, which is unused.
- Removes TRACING_DEFAULT and replaces all its uses with TRACING_EVENT, because
there is no difference in how those two are used.
- Removes TRACING_TIMESTAMP, which is unused and also doesn't do anything
different to TRACING_EVENT.
--HG--
extra : rebase_source : 69af1c53aa918798d8050e6b9d1a2658a0902af5
This patch simplifies and increases the consistency of how HAVE_NATIVE_UNWIND
is used.
- Its definition is moved from platform.h to platform.cpp, because the latter
is the only file that uses it.
- It's now defined in the same place as USE_{NS,EHABI,LUL}_STACKWALK, and used
in preference to those, where possible. Also, it's now defined on Linux and
Android even if MOZ_PROFILING is not.
- HAVE_NATIVE_UNWIND is now used consistently and by itself for all relevant
conditions, including when defining the presence and use of the "stackwalk"
feature.
- The patch inlines and removes is_native_unwinding_avail().
Note that MOZ_PROFILING must be defined for HAVE_NATIVE_UNWIND to be true on
Windows and Mac, but not on Linux and Android.
--HG--
extra : rebase_source : 5be3e5fe65706a15179a2cf46ba9451f68fff815
Bug 1339695 part 8 unintentionally changed behaviour in profiler_init() when
MOZ_PROFILING is undefined. This patch undoes that change.
--HG--
extra : rebase_source : 16e992382e06fbc673555c87499c236e2b39bc7f
This change renames the following Marionette preferences:
marionette.defaultPrefs.enabled marionette.enabled
marionette.defaultPrefs.port marionette.port
marionette.force-local marionette.forcelocal
marionette.logging marionette.log.level
The old preference names are still usable, but are henceforth considered
deprecated. They will be removed when Firefox 55 ships.
It also registers these preferences in Firefox so that they are
discoverable through about:config.
This patch also refactors testing/marionette/components/marionette.js.
MozReview-Commit-ID: 1dAMQS2e0og
--HG--
extra : rebase_source : e59a9e656442d8e8cebeaf5e741dc8306436bc22
No functional changes apart from class'ifying the file and harmonising
the export symbol with the rest of the Marionette code base by renaming
the MarionetteServer class to server.TCPListener.
MozReview-Commit-ID: 8WJKoprGGvr
--HG--
extra : rebase_source : 1f706a0f5e4c9fdccac90bfc14f767763ec85257
This patch does the following.
- Renames TickSample's members to mFoo style.
- Changes TickSample's constructor to set mTimeStamp.
- Moves TickSample creation from
SamplerThread::SuspendAndSampleAndResumeThread() to SamplerThread::Run(), so
it's not repeated for each platform.
- Changes TickSample::PopulateContext() so it takes a |tick_context_t*|
parameter, which avoids having to cast from |void*|.
Bug 1339695 part 8 accidentally disabled native stack walking on Android by
using GP_arm_android instead of GP_PLAT_arm_android in a #if. This patch fixes
that. It also fixes a couple of compile errors that crept into the relevant
code while it was disabled.
--HG--
extra : rebase_source : a7a94b018b8de7a7ca3c621a2b662859a65e69c1
This patch adds logging to some important functions that currently lack it.
Thread registration/unregistration is done with DEBUG_LOG because it's
more verbose than the other profiler logging, but less verbose than LUL's
logging.
The patch also scraps the BEGIN/END logging pairs because they bloat the output
for little gain. Now it just logs on function entry.
--HG--
extra : rebase_source : 3ef3d263c19cda03198e8b3a9ab89866f74ed1cd
The profiler will use level 3 (Info) and 4 (Debug) logging, though this patch
only uses level 3. LUL will use level 5 (Verbose) debugging.
The patch also tweaks parts of the the usage message, including adding
MOZ_PROFILER_{STARTUP,SHUTDOWN} to it.
--HG--
extra : rebase_source : f43a023912fbce993ed367cdd26b8f25f25381de
It's a very general mechanism for replacing the implementation of
printf_stderr().
It's primarily used by the profiler, sparingly, and not in an important way.
Worse, it prevents us from using MOZ_LOG in the profiler, which is something I
want. Because if any code that locks gPSMutex also calls MOZ_LOG, that then
calls printf_stderr(), which calls profiler_log(), which locks gPSMutex, which
deadlocks.
The only other use of set_stderr_callback() is for the ultra-hacky,
for-local-use-only copy_stderr_to_file() function, which was added for B2G
debugging and is no longer necessary.
This patch removes set_stderr_callback() altogether.
--HG--
extra : rebase_source : d31ecb482fe5899f62dc56a38e87d91f9271bab0
All three platform-*.cpp files have similar structure, most especially for
SamplerThread::Run(), with considerable duplication. This patch factors out
the common parts into a single implementation in platform.cpp.
* The top level structure of class SamplerThread has been moved to
platform.cpp.
* The class has some target-dependent fields, relating to signal handling and
thread identity.
* There's a single implementation of Run() in platform.cpp.
* AllocPlatformData() and PlatformDataDestructor::operator() have also been
commoned up and moved into platform.cpp.
* Time units in SamplerThread have been tidied up. We now use microseconds
throughout, except in the constructor. All time interval field and variable
names incorporate the unit (microseconds/milliseconds) for clarity. The
Windows uses of such values are scaled up/down by 1000 accordingly.
* The pre-existing MacOS Run() implementation contained logic that attempted
to keep "to schedule" in the presence of inaccuracy in the actual sleep
intervals. This now applies to all targets. A couple of comments on this
code have been added.
* platform-{win32,macos,linux-android}.cpp have had their Run() methods
removed, and all other methods placed in the same sequences, to the extent
that is possible.
* In the Win32 and MacOS implementations, Thread::SampleContext has been
renamed to Thread::SuspendSampleAndResumeThread as that better describes
what it does. In the Linux/Android implementation there was no such
separate method, so one has been created.
* The three Thread::SuspendSampleAndResumeThread methods have been commented
in such a way as to emphasise their identical top level structure.
* The point in platform.cpp where platform-{win32,macos,linux-android}.cpp are
#included has been moved slightly earlier in the file, into the
SamplerThread encampment, as that seems like a better place for it.
--HG--
extra : rebase_source : 0f93e15967b810c09e645fa593dbf85f94b53a9b
ProfileBuffer::FindLastSampleOfThread currently involves a linear search
backwards through the sample buffer. Profiling showed that to be the largest
profiler cost by far, at least on Linux. Bugs 1344118 and 1344258
significantly improve the situation, collectively reducing the cost by a
factor of at least 5 and often much more. But the linear search is still
present and still dominant. The worst of it is that it's unnecessary: we
could achieve the same by recording the start point of the most recent sample
for each thread in that thread's ThreadInfo record.
This patch does exactly that, adding the type ProfileBuffer::LastSample to
store the start points. LastSample also includes the ID of the thread it
pertains to as a read-only field, as that is needed in various places.
addTag doesn't check whether we're overwriting buffer entries containing start
points. Instead, FindLastSample checks whether the entry pointed to the
LastSample it is given still contains a marker.
--HG--
extra : rebase_source : 2987ec744a5c16e8b6814abe7efb507fc7280605
Replace it with profiler_is_active() in one place, and simply remove it in the
other places. These other places are:
- Around the call to profiler_OOP_exit_profile: profiler_OOP_exit_profile
itself already checks whether the profiler is running and does nothing if
it's not.
- When handling the 'profiler-subprocess-gather' notification. This
notification is sent by the profiler because it's interested in the
profile, so there's little reason to reject it.
- In RecvProfile: If the child process sent us a profile, it did so in
response to a GatherProfile request, so chances are that we're still
interested in that response.
These changes may get us a little closer to a state where you can call
getProfileDataAsync, stop the profiler before the content process profiles
have all come in, and then still receive a response with all the profiles.
At the moment, stopping the profiler will abort the profile gathering process,
but that seems more like an accident and less like the behavior you'd want.
MozReview-Commit-ID: 2tRXC70BztJ
--HG--
extra : rebase_source : 3b2f6f51d75d5f0d439e1a815d84164a5a763603
Instead of copying and concatenating strings into an mDest buffer in
SamplerStackFramePrintfRAII, require callers to keep the string buffer alive
for the duration of the current scope, and store the pointer to the annotation
string in the ProfileEntry. During stackwalking, concatenate the label and the
annotation (separated by a space) and store the resulting string in the
profile buffer.
MozReview-Commit-ID: GEjcLrhhdvb
--HG--
extra : rebase_source : 683749421ee2122805a249cf413e882ee5f33331
sutagent is no longer built or used; devicemanagerSUT is completely
unused. After this change, devicemanagerADB is the only implementation of
devicemanager, and test harness options like --dm_trans are eliminated.
Mark Banner
Carsten "Tomcat" Book
Dan Banner
Jan Beich
Frederik Braun
Kit Cambridge
Tom Tromey
Nicholas Nethercote
Frederik Braun
Wes Kocher
Chris Peterson
Julian Seward
Mike Hommey
Markus Stange
Iris Hsiao
Paul Bignier
Sebastian Hengst
Kan-Ru Chen
Kearwood Gilbert
Joel Maher
Andrew McCreight
Bryce Van Dyk
Edouard Oger
Florian Queze
Andreas Tolfsen
Thinker K.F. Li
Luke Wagner
Geoff Brown