Bug 890240 were dropped on update leading to gfx/skia/trunk/src/utils/SkThreadUtils_pthread_linux.cpp:17:40: error: unknown type name 'cpu_set_t' static int nth_set_cpu(unsigned int n, cpu_set_t* cpuSet) { ^ gfx/skia/trunk/src/utils/SkThreadUtils_pthread_linux.cpp:35:5: error: unknown type name 'cpu_set_t' cpu_set_t parentCpuset; ^ gfx/skia/trunk/src/utils/SkThreadUtils_pthread_linux.cpp:36:60: error: use of undeclared identifier 'cpu_set_t' if (0 != pthread_getaffinity_np(pthread_self(), sizeof(cpu_set_t), &pare... ^ gfx/skia/trunk/src/utils/SkThreadUtils_pthread_linux.cpp:40:5: error: unknown type name 'cpu_set_t' cpu_set_t cpuset; ^ gfx/skia/trunk/src/utils/SkThreadUtils_pthread_linux.cpp:44:47: error: use of undeclared identifier 'cpu_set_t'; did you mean 'cpuset'? sizeof(cpu_set_t), ^~~~~~~~~ cpuset gfx/skia/trunk/src/utils/SkThreadUtils_pthread_linux.cpp:40:15: note: 'cpuset' declared here cpu_set_t cpuset; ^ 5 errors generated. gmake[1]: *** [SkThreadUtils_pthread_linux.o] Error 1

NetBSD now uses SkThreadUtils_pthread_other.cpp. Its cpuset_create() API in attachment 827385 [details] [diff] [review] should probably be in separate file unless someone figures enough macro magic to reduce ugly ifdefs. Can someone push to Try as well?

Comment on attachment 8378097 [details] [diff] [review] re-apply fix Review of attachment 8378097 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/skia/moz.build @@ -706,5 @@ > 'trunk/src/ports/SkTime_Unix.cpp', > 'trunk/src/ports/SkTLS_pthread.cpp', > 'trunk/src/utils/android/ashmem.cpp', > 'trunk/src/utils/SkThreadUtils_pthread.cpp', > - 'trunk/src/utils/SkThreadUtils_pthread_other.cpp', moz.build is generated from generate_mozbuild.py now, so you'll need to make these changes there instead, otherwise they will be dropped on the next rebase (which is ideally in 6 weeks or so). ::: gfx/skia/trunk/src/utils/SkThreadUtils_pthread_linux.cpp @@ +12,5 @@ > #include "SkThreadUtils.h" > #include "SkThreadUtils_pthread.h" > > #include <pthread.h> > +#ifdef __FreeBSD__ Why is there BSD stuff in a linux-specific file? Shouldn't this be in _other?

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #2) > ::: gfx/skia/trunk/src/utils/SkThreadUtils_pthread_linux.cpp > @@ +12,5 @@ > > #include "SkThreadUtils.h" > > #include "SkThreadUtils_pthread.h" > > > > #include <pthread.h> > > +#ifdef __FreeBSD__ > > Why is there BSD stuff in a linux-specific file? Shouldn't this be in _other? Because the API is identical except for s/cpu_set_t/cpuset_t/. This makes it easier to stay in sync on updates. And CPU_COUNT may sometimes be undefined on Linux. _other.cpp is a stub for platforms that don't implement CPU binding.

Upstream didn't like this patch: https://codereview.chromium.org/22259005/ I'm going to look at their suggestion of not building SkThreadUtils and see if that works.

This time with generate_mozbuild.py.

Only one version needs r+.

Comment on attachment 8378383 [details] [diff] [review] only stub version Looks fine to me. Let's run it through try first though to ensure we don't break anything (I don't think it will).

Comment on attachment 8378383 [details] [diff] [review] only stub version Oops, sorry, misread the patch. You should add SkThreadUtils_pthread_other to the pre-populated separated dictionary in generate_separated_sources, rather than where you currently have it.

Comment on attachment 8378382 [details] [diff] [review] re-apply bug890240 version As discussed with upstream, we have decided this isn't the right way to go.

Regen, current moz.build isn't clean output from generate_mozbuild.py. diff --git a/gfx/skia/moz.build b/gfx/skia/moz.build index 0f4b271..39f9802 100644 --- a/gfx/skia/moz.build +++ b/gfx/skia/moz.build @@ -697,6 +697,7 @@ SOURCES += [ 'trunk/src/images/SkImageRef_ashmem.cpp', 'trunk/src/ports/SkDebug_android.cpp', + 'trunk/src/ports/SkFontHost_android_old.cpp', 'trunk/src/ports/SkFontHost_cairo.cpp', 'trunk/src/ports/SkFontHost_FreeType.cpp', 'trunk/src/ports/SkFontHost_FreeType_common.cpp', @@ -707,6 +708,7 @@ 'trunk/src/utils/android/ashmem.cpp', 'trunk/src/utils/SkThreadUtils_pthread.cpp', ] + DEFINES['SK_FONTHOST_CAIRO_STANDALONE'] = 0 if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'cocoa': SOURCES += [ 'trunk/src/ports/SkDebug_stdio.cpp', @@ -833,7 +835,7 @@ if CONFIG['INTEL_ARCHITECTURE'] and CONFIG['HAVE_TOOLCHAIN_SUPPORT_MSSSE3']: DEFINES['SK_BUILD_SSSE3'] = 1 -if (CONFIG['MOZ_WIDGET_TOOLKIT'] == 'android') or (CONFIG['MOZ_WIDGET_TOOLKIT'] == 'cocoa') or CONFIG['MOZ_WIDGET_GTK']: +if (CONFIG['MOZ_WIDGET_TOOLKIT'] == 'android') or (CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gonk') or (CONFIG['MOZ_WIDGET_TOOLKIT'] == 'cocoa') or CONFIG['MOZ_WIDGET_GTK']: DEFINES['SK_FONTHOST_DOES_NOT_USE_FONTMGR'] = 1 if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'windows':

(In reply to George Wright (:gw280) from comment #8) > Comment on attachment 8378383 [details] [diff] [review] > only stub version > > Oops, sorry, misread the patch. You should add SkThreadUtils_pthread_other > to the pre-populated separated dictionary in generate_separated_sources, > rather than where you currently have it. If I put it under |common| it'd break Windows. Only XP_UNIX platforms define SK_USE_POSIX_THREADS. How to negate with dictionary?

I think you need to list the same file under every item except common and windows

Comment on attachment 8378386 [details] [diff] [review] only stub version Review of attachment 8378386 [details] [diff] [review]: ----------------------------------------------------------------- Clearing the r? flag as per previous r- comment

Why not drop setProcessorAffinity() implementation altogether. It's not used in Mozilla tree while upstream usage is limited to trunk/tests/RefCntTest.cpp trunk/tests/AtomicTest.cpp I want to avoid listing stub for every supported XP_UNIX: linux, mac, solaris, hurd, dragonfly, freebsd, gnukfreebsd, netbsd, openbsd and maybe more.

Comment on attachment 8378753 [details] [diff] [review] drop setProcessorAffinity() version Review of attachment 8378753 [details] [diff] [review]: ----------------------------------------------------------------- Yep, that makes a lot more sense :)

Land only r+ patch, don't obsolete r- one. No Try run done because builds fine on one pthread platform.

Do we need to keep the obsolete patch ?

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2d30842fedb https://hg.mozilla.org/integration/mozilla-inbound/rev/6d098abf43ad

https://hg.mozilla.org/mozilla-central/rev/e2d30842fedb https://hg.mozilla.org/mozilla-central/rev/6d098abf43ad

Bug 974335 slipped in SkThreadUtils_pthread_linux.cpp that was supposed to be in blacklist. Regen.