Port net/tools/balsa to non-Linux platforms.

Port net/tools/balsa to non-Linux platforms.

R=rch@chromium.org
BUG=none
TEST=net_unittests --gtest_filter=*Balsa*

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276394

[wtc, 2014-06-04 00:56:12]

Ryan,

I am submitting this CL for review so we have the option of continuing to use
balsa, if it is only used in test programs.

https://codereview.chromium.org/312003002/diff/20001/net/tools/balsa/balsa_en...
File net/tools/balsa/balsa_enums.h (right):

https://codereview.chromium.org/312003002/diff/20001/net/tools/balsa/balsa_en...
net/tools/balsa/balsa_enums.h:27: #undef NO_ERROR

<WinError.h> defines the NO_ERROR macro as 0L:

#define NO_ERROR 0L                                                 // dderror

[Ryan Hamilton, 2014-06-06 18:20:56]

lgtm

https://codereview.chromium.org/312003002/diff/20001/net/tools/balsa/balsa_en...
File net/tools/balsa/balsa_enums.h (right):

https://codereview.chromium.org/312003002/diff/20001/net/tools/balsa/balsa_en...
net/tools/balsa/balsa_enums.h:27: #undef NO_ERROR
On 2014/06/04 00:56:12, wtc wrote:
> 
> <WinError.h> defines the NO_ERROR macro as 0L:
> 
> #define NO_ERROR 0L                                                 // dderror

Can you add a comment this effect? Also consider wrapping it an a #if windows
(whatever the right way to say that is)

[wtc, 2014-06-10 01:00:51]

The CQ bit was checked by wtc@chromium.org

[commit-bot: I haz the power, 2014-06-10 01:03:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wtc@chromium.org/312003002/60001

[commit-bot: I haz the power, 2014-06-10 14:55:27]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-10 15:22:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-10 15:22:25]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[wtc, 2014-06-10 16:56:38]

https://codereview.chromium.org/312003002/diff/20001/net/tools/balsa/balsa_en...
File net/tools/balsa/balsa_enums.h (right):

https://codereview.chromium.org/312003002/diff/20001/net/tools/balsa/balsa_en...
net/tools/balsa/balsa_enums.h:27: #undef NO_ERROR
On 2014/06/06 18:20:56, Ryan Hamilton wrote:
> 
> Can you add a comment this effect? Also consider wrapping it an a #if windows
> (whatever the right way to say that is)

Done.

[wtc, 2014-06-10 16:56:43]

The CQ bit was checked by wtc@chromium.org

[commit-bot: I haz the power, 2014-06-10 17:02:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wtc@chromium.org/312003002/80001

[commit-bot: I haz the power, 2014-06-11 01:04:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-11 07:06:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-11 07:07:00]

Try jobs failed on following builders:
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[wtc, 2014-06-11 14:39:58]

The CQ bit was checked by wtc@chromium.org

[commit-bot: I haz the power, 2014-06-11 14:42:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wtc@chromium.org/312003002/80001

[commit-bot: I haz the power, 2014-06-11 14:44:27]

Change committed as 276394

[Nico, 2014-07-04 00:27:31]

(If the answer to the question below is "no": There's a similar intrinsic to
ffs() that's available on Windows that I can tell you about.)

https://codereview.chromium.org/312003002/diff/80001/net/tools/balsa/balsa_fr...
File net/tools/balsa/balsa_frame.cc (right):

https://codereview.chromium.org/312003002/diff/80001/net/tools/balsa/balsa_fr...
net/tools/balsa/balsa_frame.cc:585: current += (ffs(colon_msk) - 1);
I believe ffs() is only available if strings.h was included. clang/win sets
__SSE2__ but there's no strings.h header on Windows, so it can't find ffs().

Taking a step back, what is this tool for? Is it performance-sensitive? If not,
could this block (and a similar one below) just go away?

[wtc, 2014-07-07 16:50:16]

https://codereview.chromium.org/312003002/diff/80001/net/tools/balsa/balsa_fr...
File net/tools/balsa/balsa_frame.cc (right):

https://codereview.chromium.org/312003002/diff/80001/net/tools/balsa/balsa_fr...
net/tools/balsa/balsa_frame.cc:585: current += (ffs(colon_msk) - 1);

On 2014/07/04 00:27:31, Nico (away) wrote:
> I believe ffs() is only available if strings.h was included. clang/win sets
> __SSE2__ but there's no strings.h header on Windows, so it can't find ffs().
> 
> Taking a step back, what is this tool for? Is it performance-sensitive? If
not,
> could this block (and a similar one below) just go away?

Chromium uses the code in net/tools/balsa only in some (non-performance) tests.
So it is not necessary for Chromium to compile this code with the best
performance. Also, if everything goes well, we should be able to replace this
code with something else within two or three weeks.

However, if it's not hard to port this code to Windows, I can take a stab at it.
So I'd be interested in learning about the Windows equivalent of ffs(). Is it
the _BitScanForward() function?

[Nico, 2014-07-07 16:54:16]

On Mon, Jul 7, 2014 at 9:50 AM, <wtc@chromium.org> wrote:

>
> https://codereview.chromium.org/312003002/diff/80001/net/
> tools/balsa/balsa_frame.cc
> File net/tools/balsa/balsa_frame.cc (right):
>
> https://codereview.chromium.org/312003002/diff/80001/net/
> tools/balsa/balsa_frame.cc#newcode585
> net/tools/balsa/balsa_frame.cc:585: current += (ffs(colon_msk) - 1);
>
> On 2014/07/04 00:27:31, Nico (away) wrote:
>
>> I believe ffs() is only available if strings.h was included. clang/win
>>
> sets
>
>> __SSE2__ but there's no strings.h header on Windows, so it can't find
>>
> ffs().
>
>  Taking a step back, what is this tool for? Is it
>>
> performance-sensitive? If not,
>
>> could this block (and a similar one below) just go away?
>>
>
> Chromium uses the code in net/tools/balsa only in some (non-performance)
> tests. So it is not necessary for Chromium to compile this code with the
> best performance. Also, if everything goes well, we should be able to
> replace this code with something else within two or three weeks.
>
> However, if it's not hard to port this code to Windows, I can take a
> stab at it. So I'd be interested in learning about the Windows
> equivalent of ffs(). Is it the _BitScanForward() function?
>

Yes. But if you don't need the perf and this code is going away soon, maybe
the two SSE2 blocks could just be deleted?


>
> https://codereview.chromium.org/312003002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.