Remove unnecessary branches in UrlDecoder

[hughbe]

FlushBytes already checks for if _numBytes > 0 so we can remove this check at the call site

I renamed FlushBytes to FlushBytesIfNeeded to convey this semantic at the call site

/cc @stephentoub @jamesqo

[JonHanna]

LGTM

[stephentoub]

It's not necessarily a win, e.g. if it's common for the condition to be false and the callee doesn't get inlined. Do you have perf measurements for this showing impact one way or the other?

[stephentoub]

cc: @davidsh

[hughbe]

It's not necessarily a win

I don't have time tonight to do run perf measurements, but it may be more performant to keep the checks at the call site, and remove the check in the actual method implementation. Let me know what you think.

[hughbe]

t it may be more performant to keep the checks at the call site, and remove the check in the actual method implementation.

This makes more sense and should be more performant. FIxed

[stephentoub]

What's the motivation behind the change?

cc: @davidsh

[hughbe]

None, just spotted some dead code

[stephentoub]

I see. I'm ok with it if @davidsh is.

[davidsh]

You removed the if (_numBytes > 0) because the caller already checks for it? If so, you should add an Assert() here to verify the condition.

[hughbe]

done

[davidsh]

Test Innerloop Ubuntu Release Build and Test
Test Innerloop OSX Release Build and Test
Test Innerloop OSX Debug Build and Test

[davidsh]

LGTM