5 ms·

Wow ... I want to pull my eyes out now. Is this really some uber critical path in the code or did the author(s) have too much time on their hands/optimizing som

Wow ... I want to pull my eyes out now. Is this really some uber critical path in the code or did the author(s) have too much time on their hands/optimizing some other metric (e.g. promotion, proving how smart they are, attempting to get job security). I am also astounded by two things related to documentation .. it is nearly non-existent, and where it exists, seems to provide almost no context to someone other than the original authors. You posted this for a reason .. would be good to get your thoughts. edit: adding one more point. I went to a fairly high-ranked university(top-20). We would get graded based on how clear our code was, comments (both for functions and inline comments). This code would get a zero on those metrics.

emteycz 5y ago
Hmm... I can't see how the code is unreasonable, given the complexity of the task at hand. Could you tell me what could be improved about this code (code itself, not docs etc)?
- jcelerier 5y ago
  just opened a file and...ugh https://android.googlesource.com/platform/art/+/refs/heads/master/dalvikvm/dalvikvm.cc#132 https://android.googlesource.com/platform/art/+/refs/heads/m... like, just this, basically C: if (strncmp(argv[arg_idx], "-XXlib:", strlen("-XXlib:")) == 0) { ... is just so complicated and error-prone over if(std::string_view{argv[arg_idx]}.starts_with("-XXlib:")) ... (which I guess is the intent)
  - jmgao 5y ago
    That code is older than C++11, let alone C++17. Also, your proposal isn't as efficient, since it calls strlen on construction of string_view (not that cost of parsing command line arguments is going to be measurable).
    - jjnoakes 5y ago
      > your proposal isn't as efficient, since it calls strlen on construction of string_view The code it replaces also had a strlen in it.
      - jmgao 5y ago
        A strlen of a compile-time constant, which will certainly get turned into a constant by the compiler. It does not get the length of the input string.
        pjmlp 5y ago
        Certainly isn't a fact unless proven by examining the generated machine code.
        jmgao 5y ago
        This is a worthless statement. Do you examine all of your generated machine code to ensure that it generates correct code? Any compiler that you cannot trust to inline 3 for strlen("foo") is not one you should trust to compile anything, let alone modern C++.
        maccard 5y ago
        > Do you examine all of your generated machine code to ensure that it generates correct code? (not OP). I examine the output if someone tells me it's slower, or if I care about performance. I did a very rough benchmark which says in this case they're equivalent [0], however that's not necessarily representative of the places it's called in. There's definitely a compile time overhead though [1]. On my current project there's ~20k files. On an 8 core machine (as an example), adding 150ms per file would add 7 minutes onto a clean build (before optimising the builds). [0] https://quick-bench.com/q/LBarCDIigwpmXcwZSx-RgV2x4_4 https://quick-bench.com/q/LBarCDIigwpmXcwZSx-RgV2x4_4 [1] https://build-bench.com/b/OH8qf9AgEB_l-BhANV5qP_uSCys https://build-bench.com/b/OH8qf9AgEB_l-BhANV5qP_uSCys
        5y ago
  - ncmncm 5y ago
    The code is ancient, and Google (like most) sees no value in modernizing old code, particularly for non-revenue-generating product. Furthermore, Google long ago locked themselves into an idiotically anti-RAII coding standard. So, the example is of bad old code, at a place that enforces bad coding practice. The intent appears to be to suggest this is typical practice, which is not supported.
    - pjmlp 5y ago
      Google seats at ISO C++, is a major contributor to clang and LLVM, and belongs to the group of companies with largest C++ code in production. If this what a company with deep roots in C++ world is doing, what are the large majority of "dark matter" software factories doing, specially the sweetshop ones.
      - ncmncm 5y ago
        A minuscule fraction of the people coding what they call C++ at Google come to the committee meetings. Google's ancient, idiotic prohibition on RAII is not a thing the current attendees, or other employees, have a say about. Many, attending or no, would rather be coding sensibly. And, in any case, the ancient code you present is not how code is being written even at Google today. So, your point is hopelessly muddled. There are shops where people still write new C++ like it's 2011, or 2003, or 1998, or 1992, or C. There are plenty of shops where people code the best way their current production compiler allows. There are plenty where different people do some of each of those. Vanishingly few shops make an effort to rewrite ancient code according to current best practice. I guess a sweetshop company is one with Oompa-loompas.
        deleted 5y ago
        [deleted]
        pjmlp 5y ago
        One day I would appreciate to know where that perfect C++ world of yours exist in real world. I guess I need to catch up with some Twilight Zone episodes.
  - still_grokking 5y ago
    > just opened a file and...ugh Looks just like regular PHP. So what's the point? Or did I miss something? /s

emteycz 5y ago

Hmm... I can't see how the code is unreasonable, given the complexity of the task at hand. Could you tell me what could be improved about this code (code itself, not docs etc)?

jcelerier 5y ago
just opened a file and...ugh https://android.googlesource.com/platform/art/+/refs/heads/master/dalvikvm/dalvikvm.cc#132 https://android.googlesource.com/platform/art/+/refs/heads/m... like, just this, basically C: if (strncmp(argv[arg_idx], "-XXlib:", strlen("-XXlib:")) == 0) { ... is just so complicated and error-prone over if(std::string_view{argv[arg_idx]}.starts_with("-XXlib:")) ... (which I guess is the intent)
- jmgao 5y ago
  That code is older than C++11, let alone C++17. Also, your proposal isn't as efficient, since it calls strlen on construction of string_view (not that cost of parsing command line arguments is going to be measurable).
  - jjnoakes 5y ago
    > your proposal isn't as efficient, since it calls strlen on construction of string_view The code it replaces also had a strlen in it.
    - jmgao 5y ago
      A strlen of a compile-time constant, which will certainly get turned into a constant by the compiler. It does not get the length of the input string.
      - pjmlp 5y ago
        Certainly isn't a fact unless proven by examining the generated machine code.
        jmgao 5y ago
        This is a worthless statement. Do you examine all of your generated machine code to ensure that it generates correct code? Any compiler that you cannot trust to inline 3 for strlen("foo") is not one you should trust to compile anything, let alone modern C++.
        maccard 5y ago
        > Do you examine all of your generated machine code to ensure that it generates correct code? (not OP). I examine the output if someone tells me it's slower, or if I care about performance. I did a very rough benchmark which says in this case they're equivalent [0], however that's not necessarily representative of the places it's called in. There's definitely a compile time overhead though [1]. On my current project there's ~20k files. On an 8 core machine (as an example), adding 150ms per file would add 7 minutes onto a clean build (before optimising the builds). [0] https://quick-bench.com/q/LBarCDIigwpmXcwZSx-RgV2x4_4 https://quick-bench.com/q/LBarCDIigwpmXcwZSx-RgV2x4_4 [1] https://build-bench.com/b/OH8qf9AgEB_l-BhANV5qP_uSCys https://build-bench.com/b/OH8qf9AgEB_l-BhANV5qP_uSCys
        5y ago
- ncmncm 5y ago
  The code is ancient, and Google (like most) sees no value in modernizing old code, particularly for non-revenue-generating product. Furthermore, Google long ago locked themselves into an idiotically anti-RAII coding standard. So, the example is of bad old code, at a place that enforces bad coding practice. The intent appears to be to suggest this is typical practice, which is not supported.
  - pjmlp 5y ago
    Google seats at ISO C++, is a major contributor to clang and LLVM, and belongs to the group of companies with largest C++ code in production. If this what a company with deep roots in C++ world is doing, what are the large majority of "dark matter" software factories doing, specially the sweetshop ones.
    - ncmncm 5y ago
      A minuscule fraction of the people coding what they call C++ at Google come to the committee meetings. Google's ancient, idiotic prohibition on RAII is not a thing the current attendees, or other employees, have a say about. Many, attending or no, would rather be coding sensibly. And, in any case, the ancient code you present is not how code is being written even at Google today. So, your point is hopelessly muddled. There are shops where people still write new C++ like it's 2011, or 2003, or 1998, or 1992, or C. There are plenty of shops where people code the best way their current production compiler allows. There are plenty where different people do some of each of those. Vanishingly few shops make an effort to rewrite ancient code according to current best practice. I guess a sweetshop company is one with Oompa-loompas.
      - deleted 5y ago
        [deleted]
      - pjmlp 5y ago
        One day I would appreciate to know where that perfect C++ world of yours exist in real world. I guess I need to catch up with some Twilight Zone episodes.
- still_grokking 5y ago
  > just opened a file and...ugh Looks just like regular PHP. So what's the point? Or did I miss something? /s