
3.7X speedup from removing a call to sleep - nexneo
http://trac.webkit.org/changeset/117478
======
othermaciej
I am the one who originally added this sleep call to WebKit, when we first
imported TCMalloc to use as our custom allocator. It was indeed there for a
reason, but that reason is not applicable to WebKit's allocation patterns.
TCMalloc was designed for a server workload, over time we have adapted it more
to the unique needs of a browser engine. This change may help other
operations, but probably not as much as the GC benchmark in question.

~~~
invisible
The person making this change seems to have not read the original comment
either. It sounds like your change made the previous iteration of that code
faster.

~~~
othermaciej
This reminds me why I hate comments. They tell you what someone once thought
the code did. They don't actually tell you what the code does. You should
always trust actual code and actual measurements above comments.

The original author of this comment (not me, I just imported the code)
believed that it improved performance on a given server-side workload.

The person who removed the sleep, Geoff Garen (a colleague and a friend), is a
good programmer. I am confident that he read the comment before deleting it.
However, his actual performance measurement trumps the comment.

BTW, here is Geoff's snarky response on reading the posts here that mention
the comment above the sleep() call:
<[http://www.quickmeme.com/meme/3qqgwz/>](http://www.quickmeme.com/meme/3qqgwz/>).
I agree with him.

------
edw519
True story...

Client gives me a Help Desk Ticket. User claims batch job use to run in 5
minutes but now runs for hours. The logs confirm this. The commit logs show
that an offshore programmer forgot to remove a 10 second sleep command from
inside an iteration (for debugging I presume) before promoting to production.
I removed it and got a 100X improvement in throughput.

My client said that now his user loves him; what did I do to fix it so fast?

When I told him that I removed the Sleep, he said, "No! No! No! Change it to a
5 second Sleep so I have something to give him the next time he complains!"

~~~
figure8
" that an offshore programmer forgot..."

Unnecessarily mentioning a characteristic of an individual is a sign of
selfish, biased, and/or malicious thinking. Do you mention "American
programmer" whenever recounting their bugs? It appears you have an agenda of
promoting negative stereotypes, presumably in your favor. This is lame. In the
future, you'll appear more credible if you omit this bigotry from your
comments.

I am certain there are thousands of programmers, male, female, gay, straight,
onshore and offshore who are much better than you (or me.)

~~~
ryanwaggoner
Hogwash. Haven't you ever read a novel or a news story or any kind of
narrative? Details add color (uh-oh, I'm sure that'll get you riled up) and
they help the listener understand context and picture the story in their head.
How boring would life be if every time you read or heard anything negative
about someone, it only referred to them as a human?

You can't solve prejudice and bias by pretending that people's race, gender,
nationality, height, weight, shoe size, etc. don't exist. They do, they always
will, get over it.

~~~
figure8
Sure, good stories have details, but a person's choice of detail reveals how
they see the world. The comment's mention of "offshore" clearly serves no
purpose beyond encouraging stereotype-based thinking. Knowing this programmer
was offshore is as irrelevant as knowing their sex or their eye color. We've
generally (and I think rightfully) exorcised this language in reference to
other characteristics, why not this one?

In my experience, programmers who emphasize that "offshore" programmers are
bad seem insecure about their own skills and job stability. The same is true
of sexist men who go out of their way to mention when a mistake was made by a
woman but never explicitly highlight when it was a man.

~~~
powertower
I understand what you're saying, but it's not bigotry if the "stereotype" is
true.

"offshore" is synonymous with "cheap(er) labor" in the US programming world,
and is an accurate statement since that's what it's _generally_ used for
("let's send it to XXX and it will cost us 1/10 as much")...

How many times do you think outsourcing happens in the US because they are
looking for higher-quality code? 1%, 10%, 50%, 90% of the time?

"cheap(er) labor" is also synonymous with a bad product/service, because you
often get what you pay for.

Are there exceptions to all this? Yes. Sometimes you can't find good quality
coders in the US, but I doubt this is why outsourcing happens most of the
time.

He might have generalized by adding that detail ("offshore") but that's not
biggotry IMO as much as it is a flavoring.

~~~
manojlds
Offshore is, most of the times, about getting the same quality of work at
cheaper cost. How is it cheaper? Not because it is shoddy work, but the
economics in the other country work out that way. How can you equate cheaper
price = lower quality. If you are looking for people in US alone, then cheaper
= lower quality. Since another factor of the equation, the country, is
changing, that is not the case.

~~~
lawnchair_larry
_Offshore is, most of the times, about getting the same quality of work at
cheaper cost._

I see you've never worked with offshore development.

~~~
taligent
I see you've never worked with GOOD offshore development.

~~~
dromidas
Offshore also doesn't mean not American...

------
pmjordan
It's interesting how spinlocks are making a comeback in user space code. They
are widely used in kernel code, but their use has previously been discouraged
in application code. As this example illustrates, it's pretty easy to get them
wrong...

~~~
cperciva
Spin locks are discouraged in FreeBSD kernel code too, except for very low-
level areas (serial console, pmap, and interrupt handling come to mind) where
you can't safely ask the scheduler to do anything for you.

That said, our sleep locks are actually adaptive locks, so on SMP systems
they'll spin for a bit before giving up and asking to be descheduled.

~~~
planckscnst
They are encouraged in the Linux kernel in places where it is nessesary to
lock for a very short time. Using mutexes takes a bit of overhead, so they
can't give you very short sleep times; that is when spinlocks have to be used.

<http://kernel.org/doc/Documentation/spinlocks.txt>

~~~
cperciva
In FreeBSD the vast majority of sleep lock acquisitions are uncontended and
end up being a single compare-and-exchange -- very fast. Spin locks are
slightly more work, since we disable interrupts during them.

------
grimebox
Anyone wanna supply some context? I have no idea what is going on here

------
osivertsson
It was "only" on one particular benchmark, still it shows that profiling often
give you surprising results that nobody familiar with the code anticipated.

------
cpeterso
Why did they roll their own spinlock implementation in the first place? They
could encapsulate the lock primitives provided by, and optimized for, each
platform: CRITICAL_SECTIONs on Windows, futexes on Linux, and
pthread_spin_locks elsewhere. iOS and OSX have some Mach-specific spinlocks
(OSSpinLockLock?), but I don't have any experience using them.

------
zizee
So if I understand this correctly the 3.7x speedup is a speedup in garbage
collection and will have a positive effect on the speed of the browser which
is nice.

But what sort of effect will this have on page rendering? Anyone in the know
care to comment on the specifics?

edit: improvements to my shoddy wording.

~~~
osivertsson
My guess from spending only a few days with the WebKit source is that it will
not effect page rendering at all.

Possibly not desktop performance at all since I believe GC is done in a
separate thread and (from my understanding) you never wait on it, unless you
are really memory constrained.

Could make a difference on IOS where I believe you actually don't have this GC
memory scavenger thread.

Anyway, the source is quite readable and everyone working with WebKit on one
level or another could benefit from taking a look. At the very least it gives
you a feeling for the tremendous effort from a huge number of people that the
WebKit project really is.

~~~
zizee
Cool, thanks for the insight.

------
swah
Are sleeps() always necessary in multithreaded programming?

~~~
maximilianburke
Use of sleep() in multithreaded programming is a bad code smell. There are
other safer, more accurate, ways of suspending execution of the current thread
until a particular event has occurred, though they may take a little more work
to integrate into a program than sprinkling sleep's.

Spinlock implementations sometimes use sleep() calls in them to yield the
current thread to others that may be ready to run but are currently being
blocked. It's still a bad code smell though because spinlocks, at least in
user space code, are used to avoid the overhead of yielding to other threads.
There are also other locking mechanisms, like mutexes/futexes, that will
accomplish the same yielding behavior if it's really needed, like if it's
anticipated that the code waiting for the lock may be held up for a while.

------
aristidb
There was a long comment before the sleep. It seems likely that the sleep was
there for a reason.

------
jrockway
To be fair, it's a sleep(0), which I assume is just the easiest way to get the
process to context switch. I assume this is a speed improvement only when
locks become ready sooner than a context switch.

The real fix would be to make context switches faster, this is just a data-
driven tweak rather than a general solution.

------
subhro
Awesome!! I wonder when this is going to get pushed to mainstream Safari.

~~~
jarek-foksa
The patch was committed on 2012-05-17, so it should be already present in
Safari for OSX 10.8.

------
tlrobinson
Reminds me of "the speedup loop": <http://thedailywtf.com/Articles/The-
Speedup-Loop.aspx>

------
duncanwilcox
The code is, appropriately, in the WTF directory.

------
chris_wot
What's with the old comment "// Sleep for a few milliseconds", followed by the
sleep function?

------
khet
Reduced the number of lines in the code base AND improved the product? nice.

~~~
davvid
Double nice. A co-worker and I once had a joke where we wanted our code counts
for the year to be negative. No one tracks code counts, of course, but it was
a fun idea.

He eventually replaced a particularly ugly and complicated system with a much
simpler system, and thus he met his goal of going into the negative. Less code
is better code.

