Closed Bug 972135 Opened 10 years ago Closed 2 years ago

move mozilla/GuardObjects.h to be done via static analysis instead (speed up debug builds)

Categories

(Core :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1223932

People

(Reporter: froydnj, Unassigned)

References

Details

bholley had the interesting idea that we should profile debug builds, in hopes of finding obvious things that were sucking time.  If we can find enough things, maybe we can speed up end-to-end test cycles and even enable debug builds on B2G...

Anyway, I profiled some devtools tests with perf on my Linux x86-64 box and found:

  6.79%    firefox  libxul.so                    [.] js::AutoThreadSafeAccess::AutoThreadSafeAccess(js::gc::Cell const*, mozilla::detail::GuardObjectNotifier const&)
  4.99%    firefox  libpthread-2.13.so           [.] pthread_getspecific
  2.74%    firefox  libxul.so                    [.] js::AutoThreadSafeAccess::~AutoThreadSafeAccess()
  2.29%    firefox  libxul.so                    [.] js::CurrentThreadCanAccessZone(JS::Zone*)
  1.81%    firefox  libxul.so                    [.] js::CurrentThreadCanAccessRuntime(JSRuntime*)
  1.33%    firefox  libxul.so                    [.] mozilla::detail::GuardObjectNotificationReceiver::~GuardObjectNotificationReceiver()
  1.29%    firefox  libxul.so                    [.] Interpret(JSContext*, js::RunState&)
  1.19%    firefox  libplds4.so                  [.] PL_HashString
  1.16%    firefox  libxul.so                    [.] js::ObjectImpl::getClass() const
  1.15%    firefox  libnspr4.so                  [.] PR_GetCurrentThread
  1.12%    firefox  libxul.so                    [.] pthread_getspecific@plt
  1.09%    firefox  libxul.so                    [.] js::UncheckedUnwrap(JSObject*, bool, unsigned int*)
  1.08%    firefox  libpthread-2.13.so           [.] __pthread_mutex_unlock_usercnt
  1.02%    firefox  libpthread-2.13.so           [.] pthread_mutex_lock
  0.97%     python  python2.7                    [.] 0xc1094
  0.86%    firefox  firefox                      [.] arena_malloc
  0.85%    firefox  firefox                      [.] arena_dalloc
  0.84%     python  python2.7                    [.] PyEval_EvalFrameEx
  0.84%    firefox  libpthread-2.13.so           [.] pthread_mutex_trylock
  0.76%    firefox  libplds4.so                  [.] PL_HashTableRawLookup
  0.72%    firefox  libxul.so                    [.] js::gc::Cell::isAligned() const
  0.69%    firefox  libc-2.13.so                 [.] __strcmp_sse42
  0.61%    firefox  libc-2.13.so                 [.] vfprintf
  0.59%    firefox  libxul.so                    [.] _ZN7mozilla6detail31GuardObjectNotificationReceiverD1Ev@plt
  0.59%    firefox  libxul.so                    [.] js::GCMarker::processMarkStackTop(js::SliceBudget&)
  0.55%    firefox  libxul.so                    [.] GetBloatEntry(char const*, unsigned int)

The AutoThreadSafe access stuff is Linux/x86-64 specific stuff that goes through and mprotect/munprotects pages.  Pretty expensive, but since Linux/x86-64 is our most scalable test platform, it doesn't seem worth throwing a lot of effort at that.  (Though if we could find out a way to speed up 10%+ of our test time, that'd be great.)

Not sure about the CurrentThreadCanAccess stuff.  Maybe worth filing a separate bug.

PL_HashString, sigh.  And PR_GetCurrentThread does way too much work.

The GuardObjectNotifier stuff is surprising.  Note that it really should be a little higher, since we have >1% of the time spent in the destructor itself, and then another small percentage of time spent in the PLT stub (!) getting into libxul, for a grand total of 2%.

This is reasonably high, and we shouldn't really have to do this dynamically at all.  We should just be able to stick an attribute on the class--MOZ_MUST_BE_NAMED_TEMPORARY_CLASS or somesuch--and then static analysis can verify that we are always constructing named temporaries.  Poking through the AST with clang suggests that checking for this is feasible; I'm going to take a crack at writing the analysis this week.
+1 for faster debug builds.

The bug assignee didn't login in Bugzilla in the last 7 months.
:overholt, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: froydnj+bz → nobody
Flags: needinfo?(overholt)

We've already removed all guard objects in favour of our clang static analysis in bug 1223932, so I don't think there's any work left to do here.

Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(overholt)
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.