Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate TBB external search; we've had it internal for a long time #6

Closed
wants to merge 6 commits into from

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented Feb 5, 2011

It can confuse the build if there's a TBB on the system as well. Just use the internal version.

@lgritz
Copy link
Collaborator Author

lgritz commented Feb 5, 2011

It would be great if somebody using Windows could test this before we commit it to the trunk.

…lacement of the tile & file mutexes to avoid false sharing (moving them from next to each other to next to the structures they protect).
@robert-matusewicz
Copy link

Hi,

I will check if this works on Windows in weekend (hope we can wait with pulling those changes into main repo - or maybe someone else want to confirm that this works?)

@jeremyselan
Copy link
Contributor

Looks good to me. (I've confirmed this works on OSX)

@lgritz
Copy link
Collaborator Author

lgritz commented Feb 8, 2011

There's no rush to merge this, I'll wait until Robert (or anybody else who gets to it first) verifies that it works on Windows.

@brechtvl
Copy link
Contributor

I tried building this on Windows, but ran into issues.

First, it seems that tbb explicitly links to tbb.lib at the end of _tbb_windef.h, so when linking it can't find tbb.lib. After disabling that code, there were these unresolved symbols (repeated for a few other .obj files):

imagecache.obj : error LNK2019: unresolved external symbol ___TBB_machine_load8
imagecache.obj : error LNK2019: unresolved external symbol ___TBB_machine_store8
imagecache.obj : error LNK2019: unresolved external symbol ___TBB_machine_cmpswp8
imagecache.obj : error LNK2019: unresolved external symbol ___TBB_machine_fetchadd8

These appear to be implemented in asm for Windows... so I'm not sure how to proceed, is it worth it pulling that asm code in like tbb_misc.cpp?

@brechtvl
Copy link
Contributor

brechtvl commented Mar 1, 2011

It seems TBB should be off on Windows, but this gives compile errors. Added another pull request with a fix for that problem, works fine for me now.

@lgritz
Copy link
Collaborator Author

lgritz commented Mar 3, 2011

Brecht, now that I've merged your other patch (the one that fixed the 'ifdef USE_TBB' versus 'if USE_TBB' mixup), is THIS patch ok by you? Does it seem to work ok on Windows?

@brechtvl
Copy link
Contributor

brechtvl commented Mar 3, 2011

With this patch applied and USE_TBB off, it works ok. With this patch applied and USE_TBB on, it doesn't compile because of the link errors mentioned above.

Actually that same problem came up in this discussion from some time ago:
http://lists.openimageio.org/pipermail/oiio-dev-openimageio.org/2009-June/001157.html

There dropping TBB entirely on Windows is mentioned, I'm not sure why that hasn't happened yet, but now may be a good time to do it?

@lgritz
Copy link
Collaborator Author

lgritz commented Mar 5, 2011

Merged.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants