#1565 LexCPP.cxx: inline IsSpaceEquiv

Cosmetic
closed-rejected
5
2014-01-08
2013-12-17
mberchtold
No

The IsSpaceEquiv function should be inlined. I noticed during profiling that VS 12 did not inline it by default even with the /O2 compiler switch:

From:
static bool IsSpaceEquiv(...)

To:
inline bool IsSpaceEquiv(int state)

It would probably be a good idea to inline others as well:
IsStreamCommentStyle

Discussion

  • Neil Hodgson

    Neil Hodgson - 2013-12-17

    Should retain static as do not want external linkage. What test document was used and what is the measured difference in speed?

     
  • mberchtold

    mberchtold - 2013-12-17

    I would recommend to put it into a anonymous namespace instead. The compiler did not inline it with /O2 and static inline.

    I was testing the CPP lexer with a 32 MB .c file and noticed that the function was called a few million times and it wasn't inlined. This was sufficient enough for me to change it. I haven't done any actual measurements. It is clearly a micro optimization but there is no reason not to inline this function.

    I also recommend to compile the lexers with /Ox as it seems many other functions declared as inline are in the end not inlined with /O2.

     
  • Neil Hodgson

    Neil Hodgson - 2013-12-17
    • labels: --> scintilla, lexer, cpp
    • status: open --> open-rejected
    • assigned_to: Neil Hodgson
     
  • Neil Hodgson

    Neil Hodgson - 2013-12-17

    Without evidence of a measured benefit, I will not be accepting this.

     
  • Neil Hodgson

    Neil Hodgson - 2014-01-08
    • status: open-rejected --> closed-rejected
     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks