This is an archive of the discontinued LLVM Phabricator instance.

Remove object_error::success and use std::error_code() instead
Needs ReviewPublic

Authored by ruiu on Jun 8 2015, 9:09 PM.

Details

Reviewers
Bigcheese
Summary

make_error_code(object_error) is slow because object::object_category()
uses a ManagedStatic variable. But the real problem is that the function is
called too frequently. This patch uses std::error_code() instead of
object_error::success. In most cases, we return "success", so this patch
reduces number of function calls to that function.

Diff Detail

Event Timeline

ruiu updated this revision to Diff 27359.Jun 8 2015, 9:09 PM
ruiu retitled this revision from to Remove object_error::success and use std::error_code() instead.
ruiu updated this object.
ruiu edited the test plan for this revision. (Show Details)
ruiu added a reviewer: Bigcheese.
ruiu added subscribers: aaron.ballman, yaron.keren, silvas, rnk.
ruiu added a comment.Jun 8 2015, 9:10 PM

Forgot to add llvm-commits. Please ignore. I'll create a new one.

LGTM with some minor comments:

Index: include/llvm/Object/Error.h

  • include/llvm/Object/Error.h

+++ include/llvm/Object/Error.h
@@ -22,8 +22,7 @@
const std::error_category &object_category();

enum class object_error {

  • success = 0,
  • arch_not_found,

+ arch_not_found = 1,

Can we put a comment around here that says to use std::error_code() in
place of success?

invalid_file_type,
parse_failed,
unexpected_eof,

Index: lib/Object/IRObjectFile.cpp

  • lib/Object/IRObjectFile.cpp

+++ lib/Object/IRObjectFile.cpp
@@ -195,15 +195,15 @@

unsigned Index = getAsmSymIndex(Symb);
assert(Index <= AsmSymbols.size());
OS << AsmSymbols[Index].first;
  • return object_error::success;;

+ return std::error_code();;

Extra semi colon here that can be removed

}

if (Mang)
  Mang->getNameWithPrefix(OS, GV, false);
else
  OS << GV->getName();
  • return object_error::success;

+ return std::error_code();
}

~Aaron

Out of curiosity, do we have any insight into how the MS runtime implements the default constructor for std::error_code? In libcxx, it requires calling system_category() http://llvm.org/klaus/libcxx/blob/master/include/system_error#L-481 which suggests that if using std::error_code() for success is faster on windows, then the MS runtime must have a fast solution to the problem we were having with object_category().

Looks like duplicate of D10334.