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

Jemalloc : new recipe #1965

Closed
wants to merge 3 commits into from
Closed

Conversation

extrowerk
Copy link
Member

Currently untested on every platform as at least one test fails:

test_tsd_sub_thread:test/unit/tsd.c:83: Failed assertion: (data_cleanup_count) >= (3) --> 0x1 < 0x3: Cleanup function should have executed multiple times.
test_tsd_sub_thread (non-reentrant): fail
test_tsd_reincarnation (non-reentrant): pass
--- pass: 2/3, skip: 0/3, fail: 1/3 ---
<jemalloc>: test/unit/tsd.c:31: Unreachable code reached
Abort
test/test.sh: line 34: 70246 Abort $JEMALLOC_TEST_PREFIX ${t} /sources/jemalloc-5.0.1/ /sources/jemalloc-5.0.1/
Test harness error: test/unit/tsd w/ MALLOC_CONF=""
Use prefix to debug, e.g. JEMALLOC_TEST_PREFIX="gdb --args" sh test/test.sh test/unit/tsd

Copy link
Member

@fbrosson fbrosson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestions

# devel package
packageEntries devel \
$developDir \
$prefix/bin/jemalloc-config
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use

-		$prefix/bin/jemalloc-config
+		$binDir/jemalloc-config


PROVIDES_devel="
jemalloc${secondaryArchSuffix}_devel = $portVersion compat >= 5
cmd:jemalloc_config
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd write

-		cmd:jemalloc_config
+		cmd:jemalloc_config$secondaryArchSuffix


PROVIDES="
jemalloc$secondaryArchSuffix = $portVersion compat >= 5
cmd:jemalloc.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I havent looked at anything other than your recipe, but I wonder if it would not be better to rename jemalloc.sh to jemalloc and write:

-	cmd:jemalloc.sh
+	cmd:jemalloc$secondaryArchSuffix
-	cmd:jeprof
+	cmd:jeprof$secondaryArchSuffix

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just don't want the executables with _x86 postfix.


TEST()
{
make check
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you will need to pass a LIBRARY_PATH here, probably something like:

 TEST()
 {
+	LIBRARY_PATH="$sourceDir/lib${LIBRARY_PATH:+:$LIBRARY_PATH}" \
 	make check
 }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works already great, no need for more patching here.

@alaviss
Copy link
Contributor

alaviss commented Dec 27, 2017

Issue of interest: jemalloc/jemalloc#1040 (comment)

@extrowerk
Copy link
Member Author

@waddlesplash
Copy link
Member

Can you please retest this after the runtime_loader fixes?

@extrowerk
Copy link
Member Author

Will do!

@extrowerk
Copy link
Member Author

Still the same errors.

@extrowerk extrowerk closed this Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants