Bug 198920 - [PATCH] www/squid: src/ipc/mem/Segment.cc patch to prevent memory pages flushing
Summary: [PATCH] www/squid: src/ipc/mem/Segment.cc patch to prevent memory pages flushing
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: John Marino
URL:
Keywords: patch, patch-ready
Depends on:
Blocks:
 
Reported: 2015-03-26 05:09 UTC by emz
Modified: 2015-05-01 17:02 UTC (History)
2 users (show)

See Also:


Attachments
mmap() patch (1.04 KB, patch)
2015-03-26 05:09 UTC, emz
no flags Details | Diff
patch for www/squid (2.21 KB, patch)
2015-04-15 07:55 UTC, timp87
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description emz 2015-03-26 05:09:36 UTC
Created attachment 154820 [details]
mmap() patch

Squid 3.4.x is now using mmap() in order to handle SMP, it uses mmap() even if it's not using multiple workers. The patch provided is using the same technique the PostgreSQL FreeBSD port is using to fight regression - it adds a MAP_NOSYNC flag to the actual mmap() call to prevent the dirtied pages from being flushed on disk. Without this flag such pages can be flushed very soon.

I'm using this patch in my largest production for about half a year. It's working in SMP and non-SMP mode.

However, I didn't do any tests to compare the squid performance with and without this patch.
Comment 1 John Marino freebsd_committer 2015-03-27 13:15:13 UTC
Pavel, what you do think about this patch?
Comment 2 timp87 2015-03-27 13:36:10 UTC
(In reply to John Marino from comment #1)
Well, (In reply to John Marino from comment #1)

Hi, John! Well, I'm not an expert here.
I mean I know what's mmap(), dirty page or flush, but I'm not a developer to argue whether we need this patch.

(In reply to emz from comment #0)
Hi, emz@!

I see you wrote 'regression'. Could you, please, explain a bit.
What do you call regression and why? What if squid should work as it does now?
If I don't use cache I'm not affected?

Did you try to write about this change to the official squid maillist or create a PR in their bugzilla? Or it's too FreeBSD specific? I could do it if you don't have time. I could ask and see what happens.

As I see, squid-3.5 now has the same mmap() call, without MAP_NOSYNC.
Comment 3 emz 2015-03-27 13:46:30 UTC
MAP_NOSYNC is FreeBSD-specific, Linux syscall simply doesn't have this flag.
I used the "regression" term because without this flag PostgreSQL was terribly slow. On the other hand, squid isn't a database engine, it has lesser i/o density, but I though - if it still does use the mmap() call - why not.

I didn't ask any of the squid developers, but I think I can ask Amos Jeffries, one of the main squid developers.

But the main idea was "MAP_NOSYNC definitely cannot slow down squid, in fact, it's supposed to boost it up, so why not".

I will update this bug with whatever Amos will reply.
Comment 4 timp87 2015-03-27 13:50:32 UTC
(In reply to emz from comment #3)
Great!
Thank you! Hope this will be included to squid for the future.
P.S. We have a port for squid-3.5 which probably will patch current www/squid port. It's now a separate port just for testing. Hope you'll find some time to test it! ;)
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=198089
Comment 5 emz 2015-04-09 11:37:53 UTC
Well, so far Amos ignored my question, I think it's too freebsd-specific.

At this point you can leave this as is, or include this patch as an optional user-configurable (if it's not a lot of work).

Thanks anyway.
Comment 6 John Marino freebsd_committer 2015-04-14 06:26:48 UTC
I propose that this is added to squid 35 and we don't bother with the current squid.
Comment 7 timp87 2015-04-14 08:44:30 UTC
(In reply to John Marino from comment #6)
I agree. Let's wait for the 3.5 a bit and apply it.
Kurt is going to update www/squid to 3.5 in a day.


In general, I think we should wait for some time. Then make sure no one is complaining of this change. And then try to create a PR in squid's bugzilla. Then you, Eugene, or me could confirm this is great improvement and see what happens. Emails can go down in maillist, but PRs can't =)
Comment 8 timp87 2015-04-15 07:55:28 UTC
Created attachment 155608 [details]
patch for www/squid

I'd wait for a week, for example. Then apply it.
Comment 9 John Marino freebsd_committer 2015-04-29 17:34:32 UTC
this is stuck in limbo.  Does the patch apply to the current port?
Comment 10 timp87 2015-04-29 17:37:47 UTC
(In reply to John Marino from comment #9)
Mine? Yes.
Comment 11 John Marino freebsd_committer 2015-04-29 17:40:09 UTC
there's only one squid port right? :)
Comment 12 John Marino freebsd_committer 2015-04-29 17:41:31 UTC
oh, I guess you meant "your" patch, not "your" port.

I'll promote this PR.
Comment 13 timp87 2015-04-29 17:43:25 UTC
(In reply to John Marino from comment #12)
Yes, exactly. Thanks!
Comment 14 commit-hook freebsd_committer 2015-04-30 19:34:53 UTC
A commit references this bug:

Author: marino
Date: Thu Apr 30 19:34:48 UTC 2015
New revision: 385055
URL: https://svnweb.freebsd.org/changeset/ports/385055

Log:
  www/squid: Add MAP_NOSYNC flay to mmap call

  This is the same technique used by PostgreSQL to prevent dirty pages
  from flushing prematurely (performance hit).  In any case, it can't hurt
  and it's been used in production for 18 months.  Timp87 provided the 3.5
  version of squid; the port is still unmaintained.

  PR:		198920
  Submitted by:	emz (norma.perm.ru)
  concurred:	timp87 (gmail)

Changes:
  head/www/squid/Makefile
  head/www/squid/files/patch-src_DiskIO_Mmapped_MmappedFile.cc
  head/www/squid/files/patch-src_ipc_mem_Segment.cc
Comment 15 John Marino freebsd_committer 2015-04-30 19:37:27 UTC
It's done.

(I thought you were the maintainer since 3.5, wasn't that on the PR that you were going to take over?)
Comment 16 timp87 2015-05-01 11:37:04 UTC
(In reply to John Marino from comment #15)
Well, I didn't mind to make me a maintainer, but pi@ didn't do that.
Comment 17 John Marino freebsd_committer 2015-05-01 11:55:24 UTC
with your approval, I'd like to add you as maintainer now.  This port is too used to be "unmaintained"
Comment 18 timp87 2015-05-01 16:55:10 UTC
(In reply to John Marino from comment #17)
Ok
Comment 19 John Marino freebsd_committer 2015-05-01 17:02:21 UTC
It's done!