#49141 Use tcmalloc by default
Closed: fixed 3 years ago Opened 3 years ago by mreynolds.

Issue Description

Enable tcmalloc by default. Requires gperftools-libs

Package Version and Platform

This is for 1.3.6 (RHEL 7.4)


Metadata Update from @mreynolds:
- Issue assigned to mreynolds

3 years ago

Metadata Update from @mreynolds:
- Issue priority set to: 3
- Issue set to the milestone: 1.3.6.0

3 years ago

Metadata Update from @mreynolds:
- Custom field origin adjusted to Community
- Custom field type adjusted to enhancement
- Custom field version adjusted to 1.3.6

3 years ago

Metadata Update from @mreynolds:
- Issue tagged with: Performance

3 years ago

Metadata Update from @mreynolds:
- Custom field reviewstatus adjusted to review

3 years ago

Hi Mark,

2 questions for the spec file...
1) Isn't it safer to "Require" the library even if it could be installed by default?
Requires: gperftools-libs
2) It might be useful to define the global variable
%global use_tcmalloc 1
and put the enabling code in %if %{use_tcmalloc} ?

Thanks!

Hi Mark,
2 questions for the spec file...
1) Isn't it safer to "Require" the library even if it could be installed by default?
Requires: gperftools-libs

But it is required to do the "build" which is why I used BuildRequires. As long as "Requires" is sufficient I'll change it.

2) It might be useful to define the global variable
%global use_tcmalloc 1
and put the enabling code in %if %{use_tcmalloc} ?

Yeah I thought about that, but eventually decided against it. I have no problem adding it though :)

I'll start on a new patch...

Thanks!

Oh, if you have a reason to drop it, I respect it. They are my "questions". :)

Hi Mark,
2 questions for the spec file...
1) Isn't it safer to "Require" the library even if it could be installed by default?
Requires: gperftools-libs

But it is required to do the "build" which is why I used BuildRequires. As long as "Requires" is sufficient I'll change it.
I think we need both?

Sorry, Mark. My comment was not clear...

I meant to add "Requires: gperftools-libs" for the installation.
148 +%if %{use_tcmalloc}
149 +BuildRequires: gperftools-devel
1?? +Requires: gperftools-libs
150 +TCMALLOC_FLAGS="--with-tcmalloc"
151 +%endif

Oh, if you have a reason to drop it, I respect it. They are my "questions". :)

Well I add the global flag for tcmalloc in the spec file. However, I'm still unsure about BuilldRequires vs Requires. When I said that it was required to build the server it was because "make install" fails if libtcmalloc was not present on the system. But it is also definitely needed for the server after compiling.

---> I just saw your comment about Requires. Okay - that makes sense. I'll work on a new patch :)

Thank you, Mark! You have my ack.

Metadata Update from @nhosoi:
- Custom field reviewstatus adjusted to ack (was: review)

3 years ago

1c790df..0349d01 master -> master
commit 0349d01
Author: Mark Reynolds mreynolds@redhat.com
Date: Tue Feb 21 13:05:21 2017 -0500

Metadata Update from @mreynolds:
- Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1186512

3 years ago

This fixes an error in the spec file:

0349d01..82b8f9a master -> master

Metadata Update from @mreynolds:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

3 years ago
3 years ago

This fixes some issues with tcmalloc and the new NS bundling. Additionally, this resolves an issue with tcmalloc with ASAN, and tcmalloc on s390.

Metadata Update from @firstyear:
- Issue status updated to: Open (was: Closed)

3 years ago

Metadata Update from @firstyear:
- Custom field reviewstatus adjusted to review (was: ack)

3 years ago

commit f9351cf
To ssh://git@pagure.io/389-ds-base.git
a95889d..f9351cf master -> master

Metadata Update from @firstyear:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

3 years ago

JFTR, acked after successful build in brew on all arches and discussion on IRC.

Metadata Update from @vashirov:
- Custom field reviewstatus adjusted to ack (was: review)

3 years ago

Login to comment on this ticket.

Metadata