[xpressive] Performance Tuning?

[David Abrahams-3]

on Mon Jul 20 2009, "Stewart, Robert" <Robert.Stewart-AT-sig.com> wrote:

> I tried posting a file with the test inputs to this list, but it made
> my message too large and was rejected.  I didn't think it made sense
> to put such a file in the vault, so I have not put it there -- or
> anywhere -- as yet.  Lacking any other concrete idea, I may post the
> file in the vault anyway.  I will indicate as much when I do.

You can do that, or you could check it into the SVN sandbox.  If you
don't have write access there, just send a request to
boost-owner@...

--
Dave Abrahams
BoostPro Computing
http://www.boostpro.com

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Edward Grace]

>>

Dear David,

>> I've had to rip out parts that used my home brew striding_iterator
>> since there's no way to avoid dereferencing one past the end of the
>> end.
>
> A correct strided_iterator is enclosed.

I note the following from your documentation of Boost.Iterator

   "Writing standard-conforming iterators is tricky, but the need  
comes up often.".

A clear penchant for understatement I see.  ;-)

It worries me slightly that subtleties of undefined behaviour in  
pointer arithmetic could easily go unnoticed.  My unscientific poll  
of people around here turned up zero correct answers to the basic  
problem; this is the Physics department though.

Thanks for that, I will read, cogitate, deliberate and digest!

Regards,

-ed

------------------------------------------------
"No more boom and bust." -- Dr. J. G. Brown, 1997

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Edward Grace]

>> I think the gcc
>> implementation of the stl is unchecked by  default.
>
> Yes, you can #define _GLIBCXX_DEBUG to get checking for GCC

Again, thanks.  I spotted that in the source.

That one's going to be used a lot from now on!

-ed

------------------------------------------------
"No more boom and bust." -- Dr. J. G. Brown, 1997

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

Joel de Guzman wrote:
> Stewart, Robert wrote:
>
> > When* I have the opportunity, I will extract the code that
> > exists in different files and libraries, and craft a custom
> > version of the scaffolding necessary to make my Xpressive and
> > custom code compile and which will reflect what I was
> > actually running when I gave comparative results.
>
> Thank you, Robert.

Alright folks.  I've finally extracted all of the necessary pieces and assembled them into a single main.cpp plus test inputs file.  You can find them here:

   http://tinyurl.com/price-parsing-7z

I have not supplied any build files as our local build system is unique and I don't know bjam.

The structure of main.cpp is as follows, with each section delimited by comments with lots of vertical pipes:

  0. A hopefully portable definition of int64_t extracted from boost/cstdint.hpp (is there a better option for this that I missed?)

  1. Forward declarations of the three parse functions (with functors to permit injecting the parse functions into the run_test function template)

  2. Some core and testing namespace forward declarations for main()

  3. main()

  4. More core declarations needed for the custom string and Xpressive variants

  5. core and testing namespace function definitions

  6. Custom string parser implementation

  7. Xpressive parser implementation

  8. Spirit parser stub

I put include directives needed for main(), the test harness, and the core namespace at the top.  Others, peculiar to a particular parser appear in that parser's section of the file.

The custom, string parser and my final Xpressive parser are included.  Note that xpressive_parsing::match_s is commented out and that xpressive_parsing::parse()'s match_s is a function local static because I don't have Boost.Threads built locally.  Remove the function local static and uncomment the match_s lines to use TLS for match_s, a necessary condition for a fair test.

I have not supplied a Spirit v2 implementation.  Someone else will need to provide that.  Note that the code must be reentrant.  When I was working this problem, I was constrained to use Boost 1.37.  It would be useful to know if there is a performance difference between Boost 1.37 and 1.39 for Xpressive and Spirit v2.

main() takes two arguments: the pathname of the test inputs and the number of times to run through the inputs.  On my system, parsing all of the inputs once took less than a second.  When I ran my code, with 100 iterations, the Xpressive version, with the function local static match_s, was 9X slower than the custom code.  That is worse than I saw in my more trivial tests previously.

The test inputs are arranged one test to a line, with the input to parse quoted and whitespace separated from the expected int64_t result.

Let the optimizing begin!

_____
Rob Stewart                           robert.stewart@...
Software Engineer, Core Software      using std::disclaimer;
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[OvermindDL1]

On Mon, Jul 27, 2009 at 1:34 PM, Stewart, Robert<Robert.Stewart@...> wrote:

> Joel de Guzman wrote:
>> Stewart, Robert wrote:
>>
>> > When* I have the opportunity, I will extract the code that
>> > exists in different files and libraries, and craft a custom
>> > version of the scaffolding necessary to make my Xpressive and
>> > custom code compile and which will reflect what I was
>> > actually running when I gave comparative results.
>>
>> Thank you, Robert.
>
> Alright folks.  I've finally extracted all of the necessary pieces and assembled them into a single main.cpp plus test inputs file.  You can find them here:
>
>   http://tinyurl.com/price-parsing-7z

Awesome.  I will integrate my Spirit2.1 version into it and run the
test and all so forth as well.  Once I confirm that my Spirit2.1
version tests correct then I will run the timings.


On Mon, Jul 27, 2009 at 1:34 PM, Stewart, Robert<Robert.Stewart@...> wrote:
>
> I have not supplied any build files as our local build system is unique and I don't know bjam.

I will just dump it all into Visual Studio and let it munch on it.


On Mon, Jul 27, 2009 at 1:34 PM, Stewart, Robert<Robert.Stewart@...> wrote:
> The structure of main.cpp is as follows, with each section delimited by comments with lots of vertical pipes:
>
>  0. A hopefully portable definition of int64_t extracted from boost/cstdint.hpp (is there a better option for this that I missed?)

That is what is supposed to be used for multi-platform integers with
64-bit width.  :)


On Mon, Jul 27, 2009 at 1:34 PM, Stewart, Robert<Robert.Stewart@...> wrote:

>  1. Forward declarations of the three parse functions (with functors to permit injecting the parse functions into the run_test function template)
>
>  2. Some core and testing namespace forward declarations for main()
>
>  3. main()
>
>  4. More core declarations needed for the custom string and Xpressive variants
>
>  5. core and testing namespace function definitions
>
>  6. Custom string parser implementation
>
>  7. Xpressive parser implementation
>
>  8. Spirit parser stub

I will duplicate the spirit stub three times so I can put in all three
version I made for proper comparison.


On Mon, Jul 27, 2009 at 1:34 PM, Stewart, Robert<Robert.Stewart@...> wrote:
> I put include directives needed for main(), the test harness, and the core namespace at the top.  Others, peculiar to a particular parser appear in that parser's section of the file.
>
> The custom, string parser and my final Xpressive parser are included.  Note that xpressive_parsing::match_s is commented out and that xpressive_parsing::parse()'s match_s is a function local static because I don't have Boost.Threads built locally.  Remove the function local static and uncomment the match_s lines to use TLS for match_s, a necessary condition for a fair test.
>
> I have not supplied a Spirit v2 implementation.  Someone else will need to provide that.  Note that the code must be reentrant.  When I was working this problem, I was constrained to use Boost 1.37.  It would be useful to know if there is a performance difference between Boost 1.37 and 1.39 for Xpressive and Spirit v2.

I will supply the Spirit2.1 implementation, due note that regardless
of your version of boost, it will require that you pull Spirit out of
the boost trunk, but Spirit2.1 pulled out of trunk should work fine
with Boost 1.37 and up.  If you wish I could bcp it out though, but
that would add a ton of files to this simple test.  :)


On Mon, Jul 27, 2009 at 1:34 PM, Stewart, Robert<Robert.Stewart@...> wrote:
> main() takes two arguments: the pathname of the test inputs and the number of times to run through the inputs.  On my system, parsing all of the inputs once took less than a second.  When I ran my code, with 100 iterations, the Xpressive version, with the function local static match_s, was 9X slower than the custom code.  That is worse than I saw in my more trivial tests previously.

I might also duplicate the xpressive test too, one for threadsafe, one
not.  I do not know xpressive well though so it will be a mostly
simple copy.

I am curious to see how well Spirit2.1 handles this.


On Mon, Jul 27, 2009 at 1:34 PM, Stewart, Robert<Robert.Stewart@...> wrote:
> The test inputs are arranged one test to a line, with the input to parse quoted and whitespace separated from the expected int64_t result.
>
> Let the optimizing begin!

Hear hear!

I should be able to get to it tonight after work.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[OvermindDL1]

On Mon, Jul 27, 2009 at 6:17 PM, OvermindDL1<overminddl1@...> wrote:
> /* snip */

I did a quick first test at work, just a quick compile, got some
errors, and quite frankly I do not know how this compiled in gcc
either.  First error is:
1>r:\programming_projects\spirit_price\price_parsing\main.cpp(545) :
error C2373: '_input' : redefinition; different type modifiers
1>        r:\programming_projects\spirit_price\price_parsing\main.cpp(544)
: see declaration of '_input'

The relevant code is:
   template <class T>
   T
   extract(char const * & _input, char const * _description,
      std::string const & _input);

Why do the first and last function params have the same name (_input)?
 And which one is the real input?  Based upon line 566, I changed the
last _input to _value and that error (and one other) is now gone.
Hmm, actually the third error is gone too.  Now I am getting lots of
Warnings (as errors since I by default have warnings treated as
errors) about double to int64_t cast, both in  your normal code on
line 730

Also, I added a:
   tests.reserve(450000);
right before the load_tests call, that changed the load_tests time
from like 10 seconds to about 2 seconds on my system.

Also, why are you using time(0), that only has second accuracy?
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[OvermindDL1]

On Mon, Jul 27, 2009 at 10:34 PM, OvermindDL1<overminddl1@...> wrote:

> On Mon, Jul 27, 2009 at 6:17 PM, OvermindDL1<overminddl1@...> wrote:
>> /* snip */
>
> I did a quick first test at work, just a quick compile, got some
> errors, and quite frankly I do not know how this compiled in gcc
> either.  First error is:
> 1>r:\programming_projects\spirit_price\price_parsing\main.cpp(545) :
> error C2373: '_input' : redefinition; different type modifiers
> 1>        r:\programming_projects\spirit_price\price_parsing\main.cpp(544)
> : see declaration of '_input'
>
> The relevant code is:
>   template <class T>
>   T
>   extract(char const * & _input, char const * _description,
>      std::string const & _input);
>
> Why do the first and last function params have the same name (_input)?
>  And which one is the real input?  Based upon line 566, I changed the
> last _input to _value and that error (and one other) is now gone.
> Hmm, actually the third error is gone too.  Now I am getting lots of
> Warnings (as errors since I by default have warnings treated as
> errors) about double to int64_t cast, both in  your normal code on
> line 730
>
> Also, I added a:
>   tests.reserve(450000);
> right before the load_tests call, that changed the load_tests time
> from like 10 seconds to about 2 seconds on my system.
>
> Also, why are you using time(0), that only has second accuracy?
>

Okay, I basically just copy/pasted my thread-safe version of my spirit
parser over and ran it, it returned bad parse with like 13/9 or
something like that.  According to the documentation in the original
cpp file, only "1", "1 2/3", or "1.2" are valid, not "2/3", so I
changed it to support that and ran it again, it parsed successfully
with all numbers in your tests matching successfully.  Here is what it
printed, using the horribly inaccurate time function:
Testing string-based parsing
Testing Xpressive-based parsing
Testing Spirit-based parsing
string parsing: 8s
xpressive parsing: 33s
spirit parsing: 6s

If you do not mind, I am going to add a millisecond accuracy testing
framework (test.hpp from the boost examples) to the file and change
all the nasty time calls to it for a more reliable reading.  I backed
up the main.cpp with my spirit addition before I will edit it anyway.
I am not home yet, will add the timing functionality when I get home.
I will post my new results with the enhanced testing when I get home
and finish it (maybe an hour) as well as posting both the main.cpp and
the testing header.  For now, I have attached my modified main.cpp
that includes the thread-safe spirit parser (I have not yet added the
other two, do you want me to even bother?), and it includes the single
line add in the main function "tests.reserve(150000);", which reserves
just enough memory for your test file, now the loading function takes
about half a second on my computer, instead of 9 seconds.

Anyone else want to try the attached file and report the results as
well as what platform and OS?  I am on Windows XP using MSVC8 SP1.

P.S.  I would be quite happy if anyone could get rid of that
freakishly long double->int64_t cast warning in the xpressive code, I
like clean builds.  :)


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[OvermindDL1]

On Mon, Jul 27, 2009 at 10:34 PM, OvermindDL1<overminddl1@...> wrote:

> On Mon, Jul 27, 2009 at 6:17 PM, OvermindDL1<overminddl1@...> wrote:
>> /* snip */
>
> I did a quick first test at work, just a quick compile, got some
> errors, and quite frankly I do not know how this compiled in gcc
> either.  First error is:
> 1>r:\programming_projects\spirit_price\price_parsing\main.cpp(545) :
> error C2373: '_input' : redefinition; different type modifiers
> 1>        r:\programming_projects\spirit_price\price_parsing\main.cpp(544)
> : see declaration of '_input'
>
> The relevant code is:
>   template <class T>
>   T
>   extract(char const * & _input, char const * _description,
>      std::string const & _input);
>
> Why do the first and last function params have the same name (_input)?
>  And which one is the real input?  Based upon line 566, I changed the
> last _input to _value and that error (and one other) is now gone.
> Hmm, actually the third error is gone too.  Now I am getting lots of
> Warnings (as errors since I by default have warnings treated as
> errors) about double to int64_t cast, both in  your normal code on
> line 730
>
> Also, I added a:
>   tests.reserve(450000);
> right before the load_tests call, that changed the load_tests time
> from like 10 seconds to about 2 seconds on my system.
>
> Also, why are you using time(0), that only has second accuracy?
>

The mailing list seems to be taking a very long while to send the
message, so here it is again, but the attachment in the main.cpp file
only, not the test_inputs.dat file (which, when zipped, is over
350kb).  So get the test_inputs.dat from the link in the post prior to
mine, and use the main.cpp that is attached to this post.  Here is the
message I sent as well, perhaps it will come through eventually:

Okay, I basically just copy/pasted my thread-safe version of my spirit
parser over and ran it, it returned bad parse with like 13/9 or
something like that.  According to the documentation in the original
cpp file, only "1", "1 2/3", or "1.2" are valid, not "2/3", so I
changed it to support that and ran it again, it parsed successfully
with all numbers in your tests matching successfully.  Here is what it
printed, using the horribly inaccurate time function:
Testing string-based parsing
Testing Xpressive-based parsing
Testing Spirit-based parsing
string parsing: 8s
xpressive parsing: 33s
spirit parsing: 6s

If you do not mind, I am going to add a millisecond accuracy testing
framework (test.hpp from the boost examples) to the file and change
all the nasty time calls to it for a more reliable reading.  I backed
up the main.cpp with my spirit addition before I will edit it anyway.
I am not home yet, will add the timing functionality when I get home.
I will post my new results with the enhanced testing when I get home
and finish it (maybe an hour) as well as posting both the main.cpp and
the testing header.  For now, I have attached my modified main.cpp
that includes the thread-safe spirit parser (I have not yet added the
other two, do you want me to even bother?), and it includes the single
line add in the main function "tests.reserve(150000);", which reserves
just enough memory for your test file, now the loading function takes
about half a second on my computer, instead of 9 seconds.

Anyone else want to try the attached file and report the results as
well as what platform and OS?  I am on Windows XP using MSVC8 SP1.

P.S.  I would be quite happy if anyone could get rid of that
freakishly long double->int64_t cast warning in the xpressive code, I
like clean builds.  :)


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Edward Grace]

On 28 Jul 2009, at 08:16, OvermindDL1 wrote:

> On Mon, Jul 27, 2009 at 10:34 PM,  
> OvermindDL1<overminddl1@...> wrote:
>> On Mon, Jul 27, 2009 at 6:17 PM,  
>> OvermindDL1<overminddl1@...> wrote:
>>> /* snip */
>>
>> I did a quick first test at work, just a quick compile, got some
>> errors, and quite frankly I do not know how this compiled in gcc
>> either.  First error is:
>> 1>r:\programming_projects\spirit_price\price_parsing\main.cpp(545) :
>> error C2373: '_input' : redefinition; different type modifiers
>> 1>        r:\programming_projects\spirit_price\price_parsing
>> \main.cpp(544)
>> : see declaration of '_input'
>>
>> The relevant code is:
>>   template <class T>
>>   T
>>   extract(char const * & _input, char const * _description,
>>      std::string const & _input);
>>
>> Why do the first and last function params have the same name  
>> (_input)?
>>  And which one is the real input?  Based upon line 566, I changed the
>> last _input to _value and that error (and one other) is now gone.
>> Hmm, actually the third error is gone too.  Now I am getting lots of
>> Warnings (as errors since I by default have warnings treated as
>> errors) about double to int64_t cast, both in  your normal code on
>> line 730
>>
>> Also, I added a:
>>   tests.reserve(450000);
>> right before the load_tests call, that changed the load_tests time
>> from like 10 seconds to about 2 seconds on my system.
>>
>> Also, why are you using time(0), that only has second accuracy?
>>
>
> The mailing list seems to be taking a very long while to send the
> message, so here it is again, but the attachment in the main.cpp file
> only, not the test_inputs.dat file (which, when zipped, is over
> 350kb).  So get the test_inputs.dat from the link in the post prior to
> mine, and use the main.cpp that is attached to this post.  Here is the
> message I sent as well, perhaps it will come through eventually:
>
> Okay, I basically just copy/pasted my thread-safe version of my spirit
> parser over and ran it, it returned bad parse with like 13/9 or
> something like that.  According to the documentation in the original
> cpp file, only "1", "1 2/3", or "1.2" are valid, not "2/3", so I
> changed it to support that and ran it again, it parsed successfully
> with all numbers in your tests matching successfully.  Here is what it
> printed, using the horribly inaccurate time function:
> Testing string-based parsing
> Testing Xpressive-based parsing
> Testing Spirit-based parsing
> string parsing: 8s
> xpressive parsing: 33s
> spirit parsing: 6s
>
> If you do not mind, I am going to add a millisecond accuracy testing
> framework (test.hpp from the boost examples) to the file and change
> all the nasty time calls to it for a more reliable reading.

OvermindDL1 - does my timer functionality not work?  Can you try  
using that instead?  If it's no good please let me know - and I can  
improve it.

The whole point of the timer code is to obtain reliable confidence  
bounds for precisely this kind of optimisation application in an  
efficient manner.

It hurts seeing absolute times used to compare things without any  
idea of their precision or accuracy!

Cheers,

-ed

------------------------------------------------
"No more boom and bust." -- Dr. J. G. Brown, 1997

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[OvermindDL1]

On Tue, Jul 28, 2009 at 3:11 AM, Edward Grace<ej.grace@...> wrote:

>
> On 28 Jul 2009, at 08:16, OvermindDL1 wrote:
>
>> On Mon, Jul 27, 2009 at 10:34 PM, OvermindDL1<overminddl1@...>
>> wrote:
>>>
>>> On Mon, Jul 27, 2009 at 6:17 PM, OvermindDL1<overminddl1@...>
>>> wrote:
>>>>
>>>> /* snip */
>>>
>>> I did a quick first test at work, just a quick compile, got some
>>> errors, and quite frankly I do not know how this compiled in gcc
>>> either.  First error is:
>>> 1>r:\programming_projects\spirit_price\price_parsing\main.cpp(545) :
>>> error C2373: '_input' : redefinition; different type modifiers
>>> 1>
>>>  r:\programming_projects\spirit_price\price_parsing\main.cpp(544)
>>> : see declaration of '_input'
>>>
>>> The relevant code is:
>>>  template <class T>
>>>  T
>>>  extract(char const * & _input, char const * _description,
>>>     std::string const & _input);
>>>
>>> Why do the first and last function params have the same name (_input)?
>>>  And which one is the real input?  Based upon line 566, I changed the
>>> last _input to _value and that error (and one other) is now gone.
>>> Hmm, actually the third error is gone too.  Now I am getting lots of
>>> Warnings (as errors since I by default have warnings treated as
>>> errors) about double to int64_t cast, both in  your normal code on
>>> line 730
>>>
>>> Also, I added a:
>>>  tests.reserve(450000);
>>> right before the load_tests call, that changed the load_tests time
>>> from like 10 seconds to about 2 seconds on my system.
>>>
>>> Also, why are you using time(0), that only has second accuracy?
>>>
>>
>> The mailing list seems to be taking a very long while to send the
>> message, so here it is again, but the attachment in the main.cpp file
>> only, not the test_inputs.dat file (which, when zipped, is over
>> 350kb).  So get the test_inputs.dat from the link in the post prior to
>> mine, and use the main.cpp that is attached to this post.  Here is the
>> message I sent as well, perhaps it will come through eventually:
>>
>> Okay, I basically just copy/pasted my thread-safe version of my spirit
>> parser over and ran it, it returned bad parse with like 13/9 or
>> something like that.  According to the documentation in the original
>> cpp file, only "1", "1 2/3", or "1.2" are valid, not "2/3", so I
>> changed it to support that and ran it again, it parsed successfully
>> with all numbers in your tests matching successfully.  Here is what it
>> printed, using the horribly inaccurate time function:
>> Testing string-based parsing
>> Testing Xpressive-based parsing
>> Testing Spirit-based parsing
>> string parsing: 8s
>> xpressive parsing: 33s
>> spirit parsing: 6s
>>
>> If you do not mind, I am going to add a millisecond accuracy testing
>> framework (test.hpp from the boost examples) to the file and change
>> all the nasty time calls to it for a more reliable reading.
>
> OvermindDL1 - does my timer functionality not work?  Can you try using that
> instead?  If it's no good please let me know - and I can improve it.
>
> The whole point of the timer code is to obtain reliable confidence bounds
> for precisely this kind of optimisation application in an efficient manner.
>
> It hurts seeing absolute times used to compare things without any idea of
> their precision or accuracy!

I have not seen how to use yours yet though, not actually looked at
the code.  To be honest, I am just lazy and using what I know involves
one search-and-replace, and two lines of code changed.  >.>

I will make another set with your ejg timer now since I have time for
once to play a bit.  :)

For now, I made one using the high_resolution_timer.hpp that comes
with boost for examples and benchmarks and such.  Attached is a zip of
main.cpp and the high_resolution_timer.hpp (not the test data, it is
too big to attach quickly).  Here are the results on my computer:
Testing string-based parsing
Testing Xpressive-based parsing
Testing Spirit-based parsing
string parsing: 7.2406s
xpressive parsing: 29.2227s
spirit parsing: 5.07125s

Yea, a lot more accurate, but still not good for direct comparison
with other people like the ejg timer is, I shall make a modification
with that next.  :)


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[OvermindDL1]

On Tue, Jul 28, 2009 at 4:15 AM, OvermindDL1<overminddl1@...> wrote:

> On Tue, Jul 28, 2009 at 3:11 AM, Edward Grace<ej.grace@...> wrote:
>>
>> On 28 Jul 2009, at 08:16, OvermindDL1 wrote:
>>
>>> On Mon, Jul 27, 2009 at 10:34 PM, OvermindDL1<overminddl1@...>
>>> wrote:
>>>>
>>>> On Mon, Jul 27, 2009 at 6:17 PM, OvermindDL1<overminddl1@...>
>>>> wrote:
>>>>>
>>>>> /* snip */
>>>>
>>>> I did a quick first test at work, just a quick compile, got some
>>>> errors, and quite frankly I do not know how this compiled in gcc
>>>> either.  First error is:
>>>> 1>r:\programming_projects\spirit_price\price_parsing\main.cpp(545) :
>>>> error C2373: '_input' : redefinition; different type modifiers
>>>> 1>
>>>>  r:\programming_projects\spirit_price\price_parsing\main.cpp(544)
>>>> : see declaration of '_input'
>>>>
>>>> The relevant code is:
>>>>  template <class T>
>>>>  T
>>>>  extract(char const * & _input, char const * _description,
>>>>     std::string const & _input);
>>>>
>>>> Why do the first and last function params have the same name (_input)?
>>>>  And which one is the real input?  Based upon line 566, I changed the
>>>> last _input to _value and that error (and one other) is now gone.
>>>> Hmm, actually the third error is gone too.  Now I am getting lots of
>>>> Warnings (as errors since I by default have warnings treated as
>>>> errors) about double to int64_t cast, both in  your normal code on
>>>> line 730
>>>>
>>>> Also, I added a:
>>>>  tests.reserve(450000);
>>>> right before the load_tests call, that changed the load_tests time
>>>> from like 10 seconds to about 2 seconds on my system.
>>>>
>>>> Also, why are you using time(0), that only has second accuracy?
>>>>
>>>
>>> The mailing list seems to be taking a very long while to send the
>>> message, so here it is again, but the attachment in the main.cpp file
>>> only, not the test_inputs.dat file (which, when zipped, is over
>>> 350kb).  So get the test_inputs.dat from the link in the post prior to
>>> mine, and use the main.cpp that is attached to this post.  Here is the
>>> message I sent as well, perhaps it will come through eventually:
>>>
>>> Okay, I basically just copy/pasted my thread-safe version of my spirit
>>> parser over and ran it, it returned bad parse with like 13/9 or
>>> something like that.  According to the documentation in the original
>>> cpp file, only "1", "1 2/3", or "1.2" are valid, not "2/3", so I
>>> changed it to support that and ran it again, it parsed successfully
>>> with all numbers in your tests matching successfully.  Here is what it
>>> printed, using the horribly inaccurate time function:
>>> Testing string-based parsing
>>> Testing Xpressive-based parsing
>>> Testing Spirit-based parsing
>>> string parsing: 8s
>>> xpressive parsing: 33s
>>> spirit parsing: 6s
>>>
>>> If you do not mind, I am going to add a millisecond accuracy testing
>>> framework (test.hpp from the boost examples) to the file and change
>>> all the nasty time calls to it for a more reliable reading.
>>
>> OvermindDL1 - does my timer functionality not work?  Can you try using that
>> instead?  If it's no good please let me know - and I can improve it.
>>
>> The whole point of the timer code is to obtain reliable confidence bounds
>> for precisely this kind of optimisation application in an efficient manner.
>>
>> It hurts seeing absolute times used to compare things without any idea of
>> their precision or accuracy!
>
> I have not seen how to use yours yet though, not actually looked at
> the code.  To be honest, I am just lazy and using what I know involves
> one search-and-replace, and two lines of code changed.  >.>
>
> I will make another set with your ejg timer now since I have time for
> once to play a bit.  :)
>
> For now, I made one using the high_resolution_timer.hpp that comes
> with boost for examples and benchmarks and such.  Attached is a zip of
> main.cpp and the high_resolution_timer.hpp (not the test data, it is
> too big to attach quickly).  Here are the results on my computer:
> Testing string-based parsing
> Testing Xpressive-based parsing
> Testing Spirit-based parsing
> string parsing: 7.2406s
> xpressive parsing: 29.2227s
> spirit parsing: 5.07125s
>
> Yea, a lot more accurate, but still not good for direct comparison
> with other people like the ejg timer is, I shall make a modification
> with that next.  :)
>

Ew!  Warnings from the ejg files.  My build log is even more polluted!  ;-)

Made a version of it using the ejg timer now, hope I did it well
enough, mostly just a hack-in since the pre-existing system did not
fit it well, but it compiles and runs and its result is (due note, I
bumped down the default iterations from 100 to 1 so it actually
executes today):
Calibrating overhead......done
Timer overhead (t_c) ~= : 14.6667
Jitter               ~= : 0.633371

string vs Xpressive  : 296.093 320.578 334.169% faster.
Spirit vs Xpressive  : 451.527 456.264 464.239% faster.
Spirit vs string  : 25.1096 28.8328 30.6562% faster.

As you can see, string is vastly faster then Xpressive, Spirit is even
more fast then Xpressive as compared to string, and Spirit is slightly
faster (if you consider 28% to be marginally ;-) ) then string.

For those of you unfamiliar with the ejg timer, it calculates certain
factors as you can see first, it then performs multiple tests and
iterations internally that will insure a *very* high degree of
accuracy with testing.  The three numbers are, in order, "min med
max".  Even looking at the min, Spirit is still over 25% faster then
string (and that number is with an extremely high amount of
statistical confidence).  I use Boost.Bind to call the test function
so that adds a bit of overhead, which could be noticeable on the
faster thing like Spirit, so Spirit could potentially be even faster
then the above test indicates.  I would need to rewrite the whole
tests to get rid of that restriction, and it may not even be a
restriction, the compiler could have optimized it out, hmm, let me
check the disassembly, nope it is not optimized out, so the tests
could be rewritten better, perhaps I will do that later.

Attached are all the files necessary in a zip, minus the
test_inputs.dat file of course.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

OvermindDL1 wrote:
> On Mon, Jul 27, 2009 at 1:34 PM, Stewart,
> Robert<Robert.Stewart@...> wrote:
> >
> >  0. A hopefully portable definition of int64_t extracted
> > from boost/cstdint.hpp (is there a better option for this
> > that I missed?)
>
> That is what is supposed to be used for multi-platform integers with
> 64-bit width.  :)

Sorry, I meant the INT64_C macro, not the int64_t typedef.

_____
Rob Stewart                           robert.stewart@...
Software Engineer, Core Software      using std::disclaimer;
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Edward Grace]

>> Yea, a lot more accurate, but still not good for direct comparison
>> with other people like the ejg timer is, I shall make a modification
>> with that next.  :)
>>
>
> Ew!  Warnings from the ejg files.  My build log is even more  
> polluted!  ;-)

Hey - it compiles - one step at a time!  ;-)

Can you post the warnings?  Today's warnings are tomorrows errors! I  
will attack them and hopefully iron them out - I too like clean build  
logs.

> Made a version of it using the ejg timer now, hope I did it well
> enough, mostly just a hack-in since the pre-existing system did not
> fit it well, but it compiles and runs and its result is (due note, I
> bumped down the default iterations from 100 to 1 so it actually
> executes today):

You should only need 1 call.  The timer code should work out how many  
iterations it needs in order to obtain a satisfactory answer. In  
fact, going crazy, you should be able to reliably measure the speedup  
of parsing a single character! ;-)

> Calibrating overhead......done
> Timer overhead (t_c) ~= : 14.6667
> Jitter               ~= : 0.633371

If you're employing getticks from FFTW's cycle.h perhaps it's not  
returning actual clock ticks, (e.g. from a Pentium cycle counter).

On my machine (Intel Core 2 - OS X) the timer overhead is ~109 ticks  
== 109 clock cycles.

> string vs Xpressive  : 296.093 320.578 334.169% faster.
> Spirit vs Xpressive  : 451.527 456.264 464.239% faster.
> Spirit vs string  : 25.1096 28.8328 30.6562% faster

[...]

> accuracy with testing.  The three numbers are, in order, "min med
> max".

You can read, for example the bottom line as:

With a confidence of 95%, Spirit is at least 25% faster than string  
and at most 31% faster.

> Even looking at the min, Spirit is still over 25% faster then
> string (and that number is with an extremely high amount of
> statistical confidence).  I use Boost.Bind to call the test function
> so that adds a bit of overhead, which could be noticeable on the
> faster thing like Spirit, so Spirit could potentially be even faster
> then the above test indicates.

Do you think the overhead of calling through boost::bind could be  
comparable to the length of time it takes to run the function?

>   I would need to rewrite the whole
> tests to get rid of that restriction,

Looking at the following,

testing::run_tests(std::string const & _description, tests_type const  
& _tests,
    Parser _parse, unsigned const _iterations)
{
    //std::cout << "Testing " << _description << "-based parsing" <<  
std::endl;
    _parse("0"); // prime the cache
    //util::high_resolution_timer t;
    for (unsigned i(0); _iterations > i; ++i)
    {
       for (tests_type::const_iterator it(_tests.begin()), end
(_tests.end());
            it != end; ++it)
       {
          std::string const & input(it->first);
          int64_t const expected(it->second);
          int64_t const actual(_parse(input));
          if (actual != expected)
          {
             raise_parse_failed(expected, actual, input);
          }
       }
    }
    //return t.elapsed();
}

I suggest something that simply iterates over the test data but does  
not check for correctness of parsing.  Although it won't make a fat  
lot of difference in this case at least it's then consistent - you're  
timing the parsers not the tests for equality. The correctness test  
could then be done later once the timings are complete.

Does the size of the test data set matter?  In other words do you  
notice similar speedups if the test data will all fit in cache?

> and it may not even be a
> restriction, the compiler could have optimized it out, hmm, let me
> check the disassembly, nope it is not optimized out, so the tests
> could be rewritten better, perhaps I will do that later.

I'd be interested to see if it is a problem.  If it proves to be a  
significant I can add something that can account for overhead in  
'shim' functions that are used as a call-through to bind arguments.  
I sincerely doubt the overhead is great - but you never know!

Regards,

-ed

------------------------------------------------
"No more boom and bust." -- Dr. J. G. Brown, 1997

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

OvermindDL1 wrote:

>
> I did a quick first test at work, just a quick compile, got some
> errors, and quite frankly I do not know how this compiled in gcc
> either.  First error is:
> 1>r:\programming_projects\spirit_price\price_parsing\main.cpp(545) :
> error C2373: '_input' : redefinition; different type modifiers
> 1>
> r:\programming_projects\spirit_price\price_parsing\main.cpp(544)
> : see declaration of '_input'
>
> The relevant code is:
>    template <class T>
>    T
>    extract(char const * & _input, char const * _description,
>       std::string const & _input);

That was just a copy paste error in a declaration.  GCC 4.1.2 didn't complain about it for me and I have warnings up very high.  You can remove the formal parameter names, if you like.

> Why do the first and last function params have the same name (_input)?
>  And which one is the real input?  Based upon line 566, I changed the
> last _input to _value and that error (and one other) is now gone.

Yes, the third argument is _value.

> Hmm, actually the third error is gone too.  Now I am getting lots of
> Warnings (as errors since I by default have warnings treated as
> errors) about double to int64_t cast, both in  your normal code on
> line 730

I don't know where you're seeing those warnings.  I've addressed all such warnings I get from GCC 4.1.2.  My warnings are set to "-Wall -Wno-comment -Wpointer-arith -W -Wconversion -Wno-long-long."

> Also, I added a:
>    tests.reserve(450000);
> right before the load_tests call, that changed the load_tests time
> from like 10 seconds to about 2 seconds on my system.

You're assuming the full set of test inputs.  I was allowing for testing with reduced input sets.  You could read the file twice to count the number of lines and then reserve enough elements to maintain flexibility and, possible, improve performance.  However, that's done once before running all of the tests, so I certainly didn't worry about the performance of that initialization.

> Also, why are you using time(0), that only has second accuracy?

It is simple and portable.

_____
Rob Stewart                           robert.stewart@...
Software Engineer, Core Software      using std::disclaimer;
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

OvermindDL1 wrote:
> >
> Okay, I basically just copy/pasted my thread-safe version of my spirit
> parser over and ran it, it returned bad parse with like 13/9 or
> something like that.  According to the documentation in the original
> cpp file, only "1", "1 2/3", or "1.2" are valid, not "2/3", so I

>From the beginning, I specified real numbers, whole numbers, mixed numbers, and fractions.

> changed it to support that and ran it again, it parsed successfully
> with all numbers in your tests matching successfully.  Here is what it

Excellent.

> printed, using the horribly inaccurate time function:
> Testing string-based parsing
> Testing Xpressive-based parsing
> Testing Spirit-based parsing
> string parsing: 8s
> xpressive parsing: 33s
> spirit parsing: 6s

That's why I offered the iterations argument: increase by, say, 5 times and you'll have numbers you can compare a little more accurately.

> If you do not mind, I am going to add a millisecond accuracy testing
> framework (test.hpp from the boost examples) to the file and change
> all the nasty time calls to it for a more reliable reading.  I backed

I don't mind at all.  It is helpful to get better accuracy as the test needn't be run with as many iterations.

> For now, I have attached my modified main.cpp
> that includes the thread-safe spirit parser (I have not yet added the
> other two, do you want me to even bother?), and it includes the single

I don't know the difference among them, so I can't answer for including them or not.

> P.S.  I would be quite happy if anyone could get rid of that
> freakishly long double->int64_t cast warning in the xpressive code, I
> like clean builds.  :)

I don't get the warning, so you'll have to be more specific.

_____
Rob Stewart                           robert.stewart@...
Software Engineer, Core Software      using std::disclaimer;
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[OvermindDL1]

On Tue, Jul 28, 2009 at 6:32 AM, Edward Grace<ej.grace@...> wrote:

>>> Yea, a lot more accurate, but still not good for direct comparison
>>> with other people like the ejg timer is, I shall make a modification
>>> with that next.  :)
>>>
>>
>> Ew!  Warnings from the ejg files.  My build log is even more polluted!
>>  ;-)
>
> Hey - it compiles - one step at a time!  ;-)
>
> Can you post the warnings?  Today's warnings are tomorrows errors! I will
> attack them and hopefully iron them out - I too like clean build logs.

Sure, let me separate yours out from that rather bloody massive
warning that the xpressive code generates:
1>r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(468)
: warning C4267: 'initializing' : conversion from 'size_t' to
'unsigned int', possible loss of data
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(558)
: see reference to function template instantiation
'ejg::timer_result_type
&ejg::generic_timer<ticks>::measure_execution_result<_Operation>(_Operation,ejg::timer_result_type
&)' being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            _Operation=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,xpressive_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<xpressive_parsing::parser>,boost::_bi::value<unsigned
int>>>
1>        ]
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(242)
: see reference to function template instantiation 'double
ejg::generic_timer<ticks>::measure_execution_time<_OperationB>(_Operation)'
being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            _OperationB=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,xpressive_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<xpressive_parsing::parser>,boost::_bi::value<unsigned
int>>>,
1>            _Operation=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,xpressive_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<xpressive_parsing::parser>,boost::_bi::value<unsigned
int>>>
1>        ]
1>        r:\programming_projects\spirit_price\price_parsing\main.cpp(302)
: see reference to function template instantiation 'void
ejg::generic_timer<ticks>::measure_percentage_speedup<boost::_bi::bind_t<R,F,L>,boost::_bi::bind_t<R,double
(__cdecl *)(const std::string &,const testing::tests_type
&,Parser,unsigned
int),boost::_bi::list4<A1,A2,A3,A4>>>(_OperationA,_OperationB,double
&,double &,double &)' being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            R=double,
1>            F=double (__cdecl *)(const std::string &,const
testing::tests_type &,string_parsing::parser,unsigned int),
1>            L=boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<string_parsing::parser>,boost::_bi::value<unsigned
int>>,
1>            Parser=xpressive_parsing::parser,
1>            A1=boost::_bi::value<const char *>,
1>            A2=boost::_bi::value<testing::tests_type>,
1>            A3=boost::_bi::value<xpressive_parsing::parser>,
1>            A4=boost::_bi::value<unsigned int>,
1>            _OperationA=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,string_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<string_parsing::parser>,boost::_bi::value<unsigned
int>>>,
1>            _OperationB=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,xpressive_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<xpressive_parsing::parser>,boost::_bi::value<unsigned
int>>>
1>        ]
1>r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(468)
: warning C4267: 'initializing' : conversion from 'size_t' to
'unsigned int', possible loss of data
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(558)
: see reference to function template instantiation
'ejg::timer_result_type
&ejg::generic_timer<ticks>::measure_execution_result<_Operation>(_Operation,ejg::timer_result_type
&)' being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            _Operation=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,string_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<string_parsing::parser>,boost::_bi::value<unsigned
int>>>
1>        ]
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(249)
: see reference to function template instantiation 'double
ejg::generic_timer<ticks>::measure_execution_time<_OperationA>(_Operation)'
being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            _OperationA=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,string_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<string_parsing::parser>,boost::_bi::value<unsigned
int>>>,
1>            _Operation=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,string_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<string_parsing::parser>,boost::_bi::value<unsigned
int>>>
1>        ]
1>r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(468)
: warning C4267: 'initializing' : conversion from 'size_t' to
'unsigned int', possible loss of data
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(558)
: see reference to function template instantiation
'ejg::timer_result_type
&ejg::generic_timer<ticks>::measure_execution_result<_Operation>(_Operation,ejg::timer_result_type
&)' being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            _Operation=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,spirit_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<spirit_parsing::parser>,boost::_bi::value<unsigned
int>>>
1>        ]
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(249)
: see reference to function template instantiation 'double
ejg::generic_timer<ticks>::measure_execution_time<_OperationA>(_Operation)'
being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            _OperationA=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,spirit_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<spirit_parsing::parser>,boost::_bi::value<unsigned
int>>>,
1>            _Operation=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,spirit_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<spirit_parsing::parser>,boost::_bi::value<unsigned
int>>>
1>        ]
1>        r:\programming_projects\spirit_price\price_parsing\main.cpp(309)
: see reference to function template instantiation 'void
ejg::generic_timer<ticks>::measure_percentage_speedup<boost::_bi::bind_t<R,F,L>,boost::_bi::bind_t<R,double
(__cdecl *)(const std::string &,const testing::tests_type
&,Parser,unsigned
int),boost::_bi::list4<A1,A2,A3,A4>>>(_OperationA,_OperationB,double
&,double &,double &)' being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            R=double,
1>            F=double (__cdecl *)(const std::string &,const
testing::tests_type &,spirit_parsing::parser,unsigned int),
1>            L=boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<spirit_parsing::parser>,boost::_bi::value<unsigned
int>>,
1>            Parser=xpressive_parsing::parser,
1>            A1=boost::_bi::value<const char *>,
1>            A2=boost::_bi::value<testing::tests_type>,
1>            A3=boost::_bi::value<xpressive_parsing::parser>,
1>            A4=boost::_bi::value<unsigned int>,
1>            _OperationA=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,spirit_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<spirit_parsing::parser>,boost::_bi::value<unsigned
int>>>,
1>            _OperationB=boost::_bi::bind_t<double,double (__cdecl
*)(const std::string &,const testing::tests_type
&,xpressive_parsing::parser,unsigned
int),boost::_bi::list4<boost::_bi::value<const char
*>,boost::_bi::value<testing::tests_type>,boost::_bi::value<xpressive_parsing::parser>,boost::_bi::value<unsigned
int>>>
1>        ]
1>r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(468)
: warning C4267: 'initializing' : conversion from 'size_t' to
'unsigned int', possible loss of data
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(558)
: see reference to function template instantiation
'ejg::timer_result_type
&ejg::generic_timer<ticks>::measure_execution_result<_LARGE_INTEGER(__cdecl
*)(void)>(_Operation,ejg::timer_result_type &)' being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            _Operation=_LARGE_INTEGER (__cdecl *)(void)
1>        ]
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(166)
: see reference to function template instantiation 'double
ejg::generic_timer<ticks>::measure_execution_time<_LARGE_INTEGER(__cdecl
*)(void)>(_Operation)' being compiled
1>        with
1>        [
1>            ticks=ticks,
1>            _Operation=_LARGE_INTEGER (__cdecl *)(void)
1>        ]
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(92)
: see reference to function template instantiation 'void
ejg::generic_timer<ticks>::measure_infinity_time<_LARGE_INTEGER(__cdecl
*)(void)>(_Operation,double &,double &,double &,size_t)' being
compiled
1>        with
1>        [
1>            ticks=ticks,
1>            _Operation=_LARGE_INTEGER (__cdecl *)(void)
1>        ]
1>        r:\programming_projects\spirit_price\price_parsing\other_includes\ejg\timer.cpp(80)
: while compiling class template member function 'void
ejg::generic_timer<ticks>::calibrate_chrono_overhead(void)'
1>        with
1>        [
1>            ticks=ticks
1>        ]
1>        r:\programming_projects\spirit_price\price_parsing\main.cpp(279)
: see reference to class template instantiation
'ejg::generic_timer<ticks>' being compiled
1>        with
1>        [
1>            ticks=ticks
1>        ]


On Tue, Jul 28, 2009 at 6:32 AM, Edward Grace<ej.grace@...> wrote:

>> Made a version of it using the ejg timer now, hope I did it well
>> enough, mostly just a hack-in since the pre-existing system did not
>> fit it well, but it compiles and runs and its result is (due note, I
>> bumped down the default iterations from 100 to 1 so it actually
>> executes today):
>
> You should only need 1 call.  The timer code should work out how many
> iterations it needs in order to obtain a satisfactory answer. In fact, going
> crazy, you should be able to reliably measure the speedup of parsing a
> single character! ;-)

Which is how I understood it, which is why I turned it down to 1 iteration.  :)


On Tue, Jul 28, 2009 at 6:32 AM, Edward Grace<ej.grace@...> wrote:
>> Calibrating overhead......done
>> Timer overhead (t_c) ~= : 14.6667
>> Jitter               ~= : 0.633371
>
> If you're employing getticks from FFTW's cycle.h perhaps it's not returning
> actual clock ticks, (e.g. from a Pentium cycle counter).
>
> On my machine (Intel Core 2 - OS X) the timer overhead is ~109 ticks == 109
> clock cycles.

I am using the cycle.h file, and I am pretty sure it was...


On Tue, Jul 28, 2009 at 6:32 AM, Edward Grace<ej.grace@...> wrote:
> Do you think the overhead of calling through boost::bind could be comparable
> to the length of time it takes to run the function?

No, I just tested, it is negligeable


On Tue, Jul 28, 2009 at 6:32 AM, Edward Grace<ej.grace@...> wrote:
> I suggest something that simply iterates over the test data but does not
> check for correctness of parsing.  Although it won't make a fat lot of
> difference in this case at least it's then consistent - you're timing the
> parsers not the tests for equality. The correctness test could then be done
> later once the timings are complete.

I actually already did that, however I kept getting warnings about the
measure_percentage_speedup function not being able to do something
with the template arguments for the function calls, which were just
standard void(void) functions, confuzzling.  May look at that later,
almost bed time.


On Tue, Jul 28, 2009 at 6:32 AM, Edward Grace<ej.grace@...> wrote:
> Does the size of the test data set matter?  In other words do you notice
> similar speedups if the test data will all fit in cache?

His input data is a very detailed test that tests just about every
possible input, which can have different speeds for different ones, so
I think it would be a good overall test to keep and parse all 147k
values, perhaps if there was some way to test them all individually
using ejg and get a nice report?  ;-)
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[OvermindDL1]

On Tue, Jul 28, 2009 at 6:44 AM, Stewart, Robert<Robert.Stewart@...> wrote:
>> P.S.  I would be quite happy if anyone could get rid of that
>> freakishly long double->int64_t cast warning in the xpressive code, I
>> like clean builds.  :)
>
> I don't get the warning, so you'll have to be more specific.

Here are the warnings I get, there are two, the first is plain and basic:
1>r:\programming_projects\spirit_price\price_parsing\main.cpp(789) :
warning C4244: '=' : conversion from 'const boost::int64_t' to
'double', possible loss of data

Although this is with the above ejg file set I posted so the line
number might be wrong, the line of code is:        value = _result /
denominator;

The other warning is apparently a single xpressive warning about a
conversion from int64_t to double.  The line of code in main.hpp it
references is:
  const sregex price =
     *blank
     >> !sign
     >> (real | mixed_number | fraction | integer)
     >> *space;


And the long error is (this outdoes even the sizable spirit error
message ;-)  How I wish we were getting concepts in C++1x, I still do
not understand why they took it out):
Hmm, apparently too big to post on this list (over 800kb, yes, for one
error message from xpressive).  I put it in a text file, zipped it up,
and attached it, now 6kb.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

Edward Grace wrote:

>
> > Even looking at the min, Spirit is still over 25% faster then
> > string (and that number is with an extremely high amount of
> > statistical confidence).  I use Boost.Bind to call the test function
> > so that adds a bit of overhead, which could be noticeable on the
> > faster thing like Spirit, so Spirit could potentially be even faster
> > then the above test indicates.
>
> Do you think the overhead of calling through boost::bind could be
> comparable to the length of time it takes to run the function?

I don't know -- haven't looked -- where boost::bind was added or why, but any unnecessary overhead should be eliminated unless it is proven to be insignificant.

> Looking at the following,
>
> testing::run_tests(std::string const & _description,
> tests_type const
> & _tests,
>     Parser _parse, unsigned const _iterations)
> {
[snip]
> }
>
> I suggest something that simply iterates over the test data but
> does not check for correctness of parsing.  Although it won't
> make a fat lot of difference in this case at least it's then
> consistent - you're timing the parsers not the tests for
> equality.  The correctness test could then be done later once
> the timings are complete.

I should have done that from the beginning.  It would be best, I think, to run through the data once for each test, to verify the result.  Then, timing runs can be done without any checks.  Both should run from main() each time so that any optimizations are validated before considering the performance.

> Does the size of the test data set matter?  In other words do you
> notice similar speedups if the test data will all fit in cache?

Wouldn't that give less representative performance results?

_____
Rob Stewart                           robert.stewart@...
Software Engineer, Core Software      using std::disclaimer;
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Stewart, Robert]

OvermindDL1 wrote:

> On Tue, Jul 28, 2009 at 6:44 AM, Stewart,
> Robert<Robert.Stewart@...> wrote:
> >> P.S.  I would be quite happy if anyone could get rid of that
> >> freakishly long double->int64_t cast warning in the
> >> xpressive code, I like clean builds.  :)
> >
> > I don't get the warning, so you'll have to be more specific.
>
> Here are the warnings I get, there are two, the first is
> plain and basic:
> 1>r:\programming_projects\spirit_price\price_parsing\main.cpp(789) :
> warning C4244: '=' : conversion from 'const boost::int64_t' to
> 'double', possible loss of data
>
> Although this is with the above ejg file set I posted so the line
> number might be wrong, the line of code is:        value = _result /
> denominator;

Hmmm, the only warnings I ever got were from double to int64_t.  There is, of course, a precision issue between the two types, but they don't apply to this context, so simply change the line to:

   value = static_cast<double>(_result / denominator);

> The other warning is apparently a single xpressive warning about a
> conversion from int64_t to double.  The line of code in main.hpp it
> references is:
>   const sregex price =
>      *blank
>      >> !sign
>      >> (real | mixed_number | fraction | integer)
>      >> *space;
>
> And the long error is (this outdoes even the sizable spirit error
> message ;-)  How I wish we were getting concepts in C++1x, I still do
> not understand why they took it out):

There was still a lot of experimentation, discussion, and twiddling going on.  Read http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2906.pdf.  Better to get everything else out the door sooner than later.

> Hmm, apparently too big to post on this list (over 800kb, yes, for one
> error message from xpressive).  I put it in a text file, zipped it up,
> and attached it, now 6kb.

I'm not certain what is causing that warning.  I have three suggestions:

1. In xpressive_parsing::to_price_impl::operator ()(Value const &, Value const &), change "lcm.as<double>() * numerator / denominator" to "lcm.as<double>() * static_cast<double>(numerator) / static_cast<double>(denominator)" (Perhaps only one cast is actually needed)

2. In xpressive_parsing::to_price_impl::operator ()(Value const &, double), change "lcm.as<double>() * whole + _fraction" to "lcm.as<double>() * static_cast<double>(whole) + static_cast<double>(_fraction)" (Perhaps only one cast is actually needed)

3. Simplify the regex to isolate the source.  Remove ">> (real | mixed_number | fraction | integer)" first.  If that eliminates the warning, then put ">> real" back in.  If still no warning, change it to ">> (real | mixed_number)," etc.

_____
Rob Stewart                           robert.stewart@...
Software Engineer, Core Software      using std::disclaimer;
Susquehanna International Group, LLP  http://www.sig.com

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Edward Grace]

>>
>> Can you post the warnings?  Today's warnings are tomorrows errors!  
>> I will
>> attack them and hopefully iron them out - I too like clean build  
>> logs.
>
> Sure, let me separate yours out from that rather bloody massive
> warning that the xpressive code generates:
> 1>r:\programming_projects\spirit_price\price_parsing\other_includes
> \ejg\timer.cpp(468)
> : warning C4267: 'initializing' : conversion from 'size_t' to
> 'unsigned int', possible loss of data
> 1>        r:\programming_projects\spirit_price\price_parsing
> \other_includes\ejg\timer.cpp(558)
> : see reference to function template instantiation
> 'ejg::timer_result_type
> &ejg::generic_timer<ticks>::measure_execution_result<_Operation>
> (_Operation,ejg::timer_result_type

[... more reams of stuff... ]

Someone on this list mentioned something about intractable template  
warnings....
...I can't imagine what the fuss is about.

It looks like a subtle interaction with boost::bind and the expected  
form of the functor

   f()

which is marvellously non-specific about any return types.  I will  
take a look later...

>> You should only need 1 call.  The timer code should work out how many
>> iterations it needs in order to obtain a satisfactory answer. In  
>> fact, going
>> crazy, you should be able to reliably measure the speedup of  
>> parsing a
>> single character! ;-)
>
> Which is how I understood it, which is why I turned it down to 1  
> iteration.  :)

Good show.

>>> Calibrating overhead......done
>>> Timer overhead (t_c) ~= : 14.6667
>>> Jitter               ~= : 0.633371
>>
>> If you're employing getticks from FFTW's cycle.h perhaps it's not  
>> returning
>> actual clock ticks, (e.g. from a Pentium cycle counter).
>>
>> On my machine (Intel Core 2 - OS X) the timer overhead is ~109  
>> ticks == 109
>> clock cycles.
>
> I am using the cycle.h file, and I am pretty sure it was...

Ok.

>> Do you think the overhead of calling through boost::bind could be  
>> comparable
>> to the length of time it takes to run the function?
>
> No, I just tested, it is negligeable

Good!  One less thing to worry about.

> On Tue, Jul 28, 2009 at 6:32 AM, Edward  
> Grace<ej.grace@...> wrote:
>> I suggest something that simply iterates over the test data but  
>> does not
>> check for correctness of parsing.  Although it won't make a fat  
>> lot of
>> difference in this case at least it's then consistent - you're  
>> timing the
>> parsers not the tests for equality. The correctness test could  
>> then be done
>> later once the timings are complete.
>
> I actually already did that, however I kept getting warnings about the
> measure_percentage_speedup function not being able to do something
> with the template arguments for the function calls, which were just
> standard void(void) functions, confuzzling.  May look at that later,
> almost bed time.

"Confuzzling" - I like that.  Perhaps boost::bind gets up to some  
mischief.  Let me know how you get on.  I'm still trying to figure  
out why the tests I ran yielded a ~10x speedup for Spirit.

Perhaps you could try a more canonical test - running  
"ejg_uint_parser_0_0_4.cpp"

http://tinyurl.com/lro5ok

That does not make use of boost::bind - but tries to avoid the  
optimizer getting rid of void(void) functions by using globals.

>> Does the size of the test data set matter?  In other words do you  
>> notice
>> similar speedups if the test data will all fit in cache?
>
> His input data is a very detailed test that tests just about every
> possible input, which can have different speeds for different ones, so
> I think it would be a good overall test to keep and parse all 147k
> values, perhaps if there was some way to test them all individually
> using ejg and get a nice report?  ;-)

Perhaps (one day) it could be informative to test subsets of the  
data.  For example you may find Spirit is unusually slow at parsing  
certain patterns e.g. "2222" is 20% slower than "1111" - this could  
(speculatively) point towards some deep and subtle changes that could  
be made.

 From what I've seen so far however there's plenty of work to be done  
on Xpressive in closing the existing performance gap. ;-)

Cheers,

-ed
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost