List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:November 30 2010 5:00pm
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (guilhem:3393) Bug#47609
View as plain text  
On 11/30/10 11:28 AM, Guilhem Bichot wrote:
> Davi Arnaut a écrit, Le 30.11.2010 12:17:
>> On 11/30/10 8:32 AM, Guilhem Bichot wrote:
>>> Hello,
>>>
>>> Davi Arnaut a écrit, Le 30.11.2010 11:25:
>>>> Hi Guilhem,
>>>>
>>>> On 11/30/10 7:59 AM, Guilhem Bichot wrote:
>>>>> #At file:///home/mysql_src/bzrrepos_new/mysql-trunk-bugfixing/ based
>>>>> on revid:li-bing.song@stripped
>>>>>
>>>>> 3393 Guilhem Bichot 2010-11-30
>>>>> Fix for BUG#47609 "Many compile warnings are reported for Optimizer
>>>>> in PB2"
>>>>> @ CMakeLists.txt
>>>>> detect -Werror
>>>>> @ config.h.cmake
>>>>> convert COMPILE_FLAG_WERROR into #define in my_config.h
>>>>> @ sql/sql_select.cc
>>>>> do useless initializations if really needed
>>>>>
>>>>> modified:
>>>>> CMakeLists.txt
>>>>> config.h.cmake
>>>>> sql/sql_select.cc
>>>>> === modified file 'CMakeLists.txt'
>>>>> --- a/CMakeLists.txt 2010-11-24 10:35:06 +0000
>>>>> +++ b/CMakeLists.txt 2010-11-30 09:59:23 +0000
>>>>> @@ -268,6 +268,14 @@ IF(MYSQL_MAINTAINER_MODE)
>>>>> SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}
>>>>> ${MY_MAINTAINER_CXX_WARNINGS}")
>>>>> ENDIF()
>>>>>
>>>>> +IF(CMAKE_COMPILER_IS_GNUCXX)
>>>>> + STRING(REGEX MATCH "-Werror"
>>>>> + BUILD_WITH_WERROR ${CMAKE_CXX_FLAGS})
>>>>> + IF(BUILD_WITH_WERROR)
>>>>> + SET("COMPILE_FLAG_WERROR" 1)
>>>>> + ENDIF()
>>>>> +ENDIF()
>>>>> +
>>>>> IF(WITH_UNIT_TESTS)
>>>>> ENABLE_TESTING()
>>>>> ENDIF()
>>>>>
>>>>> === modified file 'config.h.cmake'
>>>>> --- a/config.h.cmake 2010-10-04 15:25:10 +0000
>>>>> +++ b/config.h.cmake 2010-11-30 09:59:23 +0000
>>>>> @@ -569,7 +569,7 @@
>>>>> #cmakedefine HAVE_CHARSET_utf32 1
>>>>> #cmakedefine HAVE_UCA_COLLATIONS 1
>>>>> #cmakedefine HAVE_COMPRESS 1
>>>>> -
>>>>> +#cmakedefine COMPILE_FLAG_WERROR 1
>>>>>
>>>>> /*
>>>>> Stuff that always need to be defined (compile breaks without it)
>>>>>
>>>>> === modified file 'sql/sql_select.cc'
>>>>> --- a/sql/sql_select.cc 2010-11-29 11:28:55 +0000
>>>>> +++ b/sql/sql_select.cc 2010-11-30 09:59:23 +0000
>>>>> @@ -6586,11 +6586,15 @@ public:
>>>>> best_loose_scan_records - same
>>>>> best_max_loose_keypart - same
>>>>> best_loose_scan_start_key - same
>>>>> - Not initializing them causes compiler warnings, but using
>>>>> UNINIT_VAR()
>>>>> - would cause a 2% CPU time loss in a 20-table plan search.
>>>>> - So, until UNINIT_VAR(x) doesn't do x=0 for any C++ code, it's not
>>>>> used
>>>>> - here.
>>>>> - */
>>>>> + Not initializing them causes compiler warnings with g++ at -O1 or
>>>>> higher,
>>>>> + but initializing them would cause a 2% CPU time loss in a 20-table
>>>>> plan
>>>>> + search. So we initialize only if warnings would stop the build.
>>>>> + */
>>>>> +#ifdef COMPILE_FLAG_WERROR
>>>>> + bound_sj_equalities= quick_max_loose_keypart= best_loose_scan_key=
>>>>> + best_loose_scan_records= best_max_loose_keypart= 0;
>>>>> + best_loose_scan_start_key= NULL;
>>>
>>> I KNEW you would reply on that one :-)
>>>
>>>> The UNINIT_VAR macro does not work here?
>>>
>>> I tried lots of things :-(
>>> This is C++, so UNINIT_VAR will do initializations in all builds (due to
>>> a never-fixed gcc bug mentioned in my_global.h). And we don't want that,
>>> because those useless initializations were measured to have an impact
>>> (see "2%" above).
>>
>> Huh, very strange. That would be roughly a few instructions. It's not
>> within variance? For example and judging from past experiences,
>> benchmark numbers with MySQL tend to vary quite a bit. When such minor
>> things make a difference, something is probably going on (either in
>> the code, or in the compiler optimizer).
>
> The scenario, very particular, was a join of 20 tables (BUG#50595). The
> optimizer spent seconds (minutes in the debug build - causing test
> timeouts in pb2, which is how it all started :-), to find the optimal
> plan because of "combinatorial explosion" of the set of possible plans;
> it ran best_access_path() millions of times. So those few instructions
> in this function run many times, gave a repeatable 2% slowdown. That's

in the test case time?

> why I did this change. I also did other changes; we had a function
> called many times, which handles subqueries and looked like
> if(false cond)
> do something;
> if(other false cond)
> do something;
> etc;
> return;
> Adding to the start of the function:
> "if (no subqueries in this query) return;"
> saved 10% of the query's total time.

but is this workload real? I mean, if its a code path that is supposed 
to be accessed millions of times, it makes sense to micro-optimize it. 
Not so much for cases of a bad query which leads to a "combinatorial 
explosion".

Regards,

Davi
Thread
bzr commit into mysql-trunk-bugfixing branch (guilhem:3393) Bug#47609Guilhem Bichot30 Nov
Re: bzr commit into mysql-trunk-bugfixing branch (guilhem:3393) Bug#47609Davi Arnaut30 Nov
  • Re: bzr commit into mysql-trunk-bugfixing branch (guilhem:3393) Bug#47609Guilhem Bichot30 Nov
    • Re: bzr commit into mysql-trunk-bugfixing branch (guilhem:3393) Bug#47609Davi Arnaut30 Nov
      • Re: bzr commit into mysql-trunk-bugfixing branch (guilhem:3393) Bug#47609Guilhem Bichot30 Nov
        • Re: bzr commit into mysql-trunk-bugfixing branch (guilhem:3393) Bug#47609Davi Arnaut30 Nov
          • Re: bzr commit into mysql-trunk-bugfixing branch (guilhem:3393) Bug#47609Davi Arnaut30 Nov