Page MenuHomePhabricator

luasandbox profiler doesn't sort results due to an HHVM bug
Closed, ResolvedPublic

Description

When trying to use Scribunto's Lua profiler on enwiki, I noticed that the functions weren't sorted in descending order by time. Some experimentation led me to discover this bit of code in HHVM:

ZEND_API int zend_hash_sort(HashTable *ht, sort_func_t sort_func,
              compare_func_t compar, int renumber TSRMLS_DC) {
  assert(sort_func == zend_qsort);
  // TODO figure out how to use compar
  ht->ksort(0, true);
  return SUCCESS;
}

In other words, instead of using the specified comparison function it just always does a ksort().

It'll probably be easiest to just work around this in Scribunto with arsort().

Event Timeline

Change 337541 had a related patch set uploaded (by Anomie):
Work around HHVM bug that prevents proper sorting from the profiler

https://gerrit.wikimedia.org/r/337541

Change 339442 had a related patch set uploaded (by Anomie):
Fix getProfilerFunctionReport sorting in HHVM

https://gerrit.wikimedia.org/r/339442

Change 337541 abandoned by Anomie:
Work around HHVM bug that prevents proper sorting from the profiler

Reason:
In favor of fixing it in luasandbox: I0a6d60fd981ab41f8b39b93f0164ee35214e6147

https://gerrit.wikimedia.org/r/337541

Change 339442 merged by jenkins-bot:
Fix getProfilerFunctionReport sorting in HHVM

https://gerrit.wikimedia.org/r/339442

Anomie edited projects, added SRE; removed Patch-For-Review.

The bug should be fixed now in the master branch of the mediawiki/php/luasandbox repo. As far as I know, to get it fixed on Wikipedia it now needs someone in Ops to update the Debian package and have puppet deploy it.

I'm triaging this as "low" priority since the Lua profiler currently needs a special URL parameter to be enabled on a request (see rOMWCb195aeec7bf5: Enable LuaSandbox profiling when `forceprofile` is true).

HHVM-Luasandbox has been upgraded to 2.0.13 in production. Bug can be closed?

Anomie claimed this task.