Skip to content

Commit

Permalink
Deprecate SbLogIsTty from Starboard 16 (#3645)
Browse files Browse the repository at this point in the history
This function is only used to turn on colored console output for
developers.
For GTest, it's a constant on already, and for automem table printing it
adds very little value, so removed.

b/243811564
  • Loading branch information
kaidokert authored Jun 24, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 49aed58 commit 8340a97
Showing 21 changed files with 44 additions and 207 deletions.
11 changes: 5 additions & 6 deletions cobalt/browser/memory_settings/auto_mem.cc
Original file line number Diff line number Diff line change
@@ -277,22 +277,21 @@ std::vector<MemorySetting*> AutoMem::AllMemorySettingsMutable() const {
return all_settings;
}

void AutoMem::LogToPrettyPrintString(const math::Size& ui_resolution,
bool use_color_ascii) {
void AutoMem::LogToPrettyPrintString(const math::Size& ui_resolution) {
#if !defined(COBALT_BUILD_TYPE_GOLD)
std::stringstream ss;

ss << "AutoMem (resolution: " << ui_resolution << "):\n";
std::vector<const MemorySetting*> all_settings = AllMemorySettings();
ss << GeneratePrettyPrintTable(use_color_ascii, all_settings);
ss << GeneratePrettyPrintTable(all_settings);

int64_t cpu_consumption =
SumMemoryConsumption(MemorySetting::kCPU, all_settings);
int64_t gpu_consumption =
SumMemoryConsumption(MemorySetting::kGPU, all_settings);

ss << GenerateMemoryTable(use_color_ascii, *max_cpu_bytes_, *max_gpu_bytes_,
cpu_consumption, gpu_consumption);
ss << GenerateMemoryTable(*max_cpu_bytes_, *max_gpu_bytes_, cpu_consumption,
gpu_consumption);

// Copy strings and optionally add more.
std::vector<std::string> error_msgs = error_msgs_;
@@ -442,7 +441,7 @@ void AutoMem::ConstructSettings(const math::Size& ui_resolution,
CheckConstrainingValues(*all_memory_settings[i]);
}

LogToPrettyPrintString(ui_resolution, SbLogIsTty());
LogToPrettyPrintString(ui_resolution);
}

} // namespace memory_settings
3 changes: 1 addition & 2 deletions cobalt/browser/memory_settings/auto_mem.h
Original file line number Diff line number Diff line change
@@ -65,8 +65,7 @@ class AutoMem {
private:
// Logs a table of the memory settings and available memory for cpu and gpu.
// This is used during startup to display memory configuration information.
void LogToPrettyPrintString(const math::Size& ui_resolution,
bool use_color_ascii);
void LogToPrettyPrintString(const math::Size& ui_resolution);

// AllMemorySettings - does not include cpu & gpu max memory.
std::vector<const MemorySetting*> AllMemorySettings() const;
11 changes: 2 additions & 9 deletions cobalt/browser/memory_settings/pretty_print.cc
Original file line number Diff line number Diff line change
@@ -63,11 +63,8 @@ void FillStream(size_t count, const char fill_ch, std::stringstream* ss) {
} // namespace

std::string GeneratePrettyPrintTable(
bool use_color_ascii, const std::vector<const MemorySetting*>& settings) {
const std::vector<const MemorySetting*>& settings) {
TablePrinter printer;
if (use_color_ascii) {
printer.set_text_color(TablePrinter::kGreen);
}
std::vector<std::string> header;
header.push_back("SETTING NAME");
header.push_back("VALUE");
@@ -96,15 +93,11 @@ std::string GeneratePrettyPrintTable(
return table_string;
}

std::string GenerateMemoryTable(bool use_color_ascii,
const IntSetting& total_cpu_memory,
std::string GenerateMemoryTable(const IntSetting& total_cpu_memory,
const IntSetting& total_gpu_memory,
int64_t settings_cpu_consumption,
int64_t settings_gpu_consumption) {
TablePrinter printer;
if (use_color_ascii) {
printer.set_text_color(TablePrinter::kGreen);
}
std::vector<std::string> header;
header.push_back("MEMORY");
header.push_back("SOURCE");
4 changes: 1 addition & 3 deletions cobalt/browser/memory_settings/pretty_print.h
Original file line number Diff line number Diff line change
@@ -43,7 +43,6 @@ namespace memory_settings {
// clang-format on

std::string GeneratePrettyPrintTable(
bool use_color_ascii,
const std::vector<const MemorySetting*>& memory_settings);

// Generates a table, ie:
@@ -58,8 +57,7 @@ std::string GeneratePrettyPrintTable(
// |______|__________|__________|
// When optional total_gpu_memory is null then the the value in the output
// table will be <UNKNOWN>.
std::string GenerateMemoryTable(bool use_color_ascii,
const IntSetting& total_cpu_memory,
std::string GenerateMemoryTable(const IntSetting& total_cpu_memory,
const IntSetting& total_gpu_memory,
int64_t settings_cpu_consumption,
int64_t settings_gpu_consumption);
9 changes: 3 additions & 6 deletions cobalt/browser/memory_settings/pretty_print_test.cc
Original file line number Diff line number Diff line change
@@ -59,7 +59,7 @@ TEST(MemorySettingsPrettyPrint, GeneratePrettyPrintTable) {
TestSettingGroup setting_group;
setting_group.LoadDefault();
std::string actual_string =
GeneratePrettyPrintTable(false, setting_group.AsConstVector());
GeneratePrettyPrintTable(setting_group.AsConstVector());

// clang-format off
EXPECT_TRUE(HasTokensInOrder(
@@ -78,8 +78,7 @@ TEST(MemorySettingsPrettyPrint, GenerateMemoryTableWithUnsetGpuMemory) {
IntSetting gpu_memory_setting("max_gpu_memory");

std::string actual_output =
GenerateMemoryTable(false, // No color.
cpu_memory_setting, // 256 MB CPU available
GenerateMemoryTable(cpu_memory_setting, // 256 MB CPU available
gpu_memory_setting,
128 * 1024 * 1024, // 128 MB CPU consumption
0); // 0 MB GPU consumption.
@@ -99,8 +98,7 @@ TEST(MemorySettingsPrettyPrint, GenerateMemoryTableWithGpuMemory) {
gpu_memory_setting.set_value(MemorySetting::kStarboardAPI, 64 * 1024 * 1024);

std::string actual_output =
GenerateMemoryTable(false, // No color.
cpu_memory_setting, // 256 MB CPU available.
GenerateMemoryTable(cpu_memory_setting, // 256 MB CPU available.
gpu_memory_setting, // 64 MB GPU available.
128 * 1024 * 1024, // 128 MB CPU consumption.
23592960); // 22.5 MB GPU consumption.
@@ -120,7 +118,6 @@ TEST(MemorySettingsPrettyPrint, GenerateMemoryWithInvalidGpuMemoryConsumption) {
gpu_memory_setting.set_value(MemorySetting::kStarboardAPI, 0);

std::string actual_output = GenerateMemoryTable(
false, // No color.
cpu_memory_setting, // 256 MB CPU available.
gpu_memory_setting, // Signals that no gpu memory is available
// on this system.
89 changes: 12 additions & 77 deletions cobalt/browser/memory_settings/table_printer.cc
Original file line number Diff line number Diff line change
@@ -28,57 +28,6 @@ namespace browser {
namespace memory_settings {
namespace {

std::string AddColor(TablePrinter::Color color, const std::string& value) {
const char* RED_START = "\x1b[31m";
const char* GREEN_START = "\x1b[32m";
const char* YELLOW_START = "\x1b[33m";
const char* BLUE_START = "\x1b[34m";
const char* MAGENTA_START = "\x1b[35m";
const char* CYAN_START = "\x1b[36m";

const char* COLOR_END = "\x1b[0m";

if (color == TablePrinter::kDefault) {
return value;
}

std::stringstream ss;
switch (color) {
case TablePrinter::kRed: {
ss << RED_START;
break;
}
case TablePrinter::kGreen: {
ss << GREEN_START;
break;
}
case TablePrinter::kYellow: {
ss << YELLOW_START;
break;
}
case TablePrinter::kBlue: {
ss << BLUE_START;
break;
}
case TablePrinter::kMagenta: {
ss << MAGENTA_START;
break;
}
case TablePrinter::kCyan: {
ss << CYAN_START;
break;
}
case TablePrinter::kDefault: {
DCHECK(false) << "Unexpected";
break;
}
}

ss << value;
ss << COLOR_END;
return ss.str();
}

std::string MakeFill(const char* str, size_t n) {
std::stringstream ss;
for (size_t i = 0; i < n; ++i) {
@@ -93,17 +42,13 @@ class TablePrinterImpl {
typedef std::vector<std::string> Row;
// table[i] gets row, table[i][j] gets row/col
typedef std::vector<Row> Table;
static std::string ToString(const Table& table,
TablePrinter::Color text_color,
TablePrinter::Color table_color);
static std::string ToString(const Table& table);

private:
static bool CheckNotEmptyAndAllColumnsTheSame(const Table& rows);
static std::vector<size_t> MakeColumnSizeVector(const Table& table);
static size_t ColumnPrintSize(const Table& table, size_t which_col);
TablePrinterImpl(const std::vector<size_t>& column_sizes,
TablePrinter::Color text_color,
TablePrinter::Color table_printer);
explicit TablePrinterImpl(const std::vector<size_t>& column_sizes);

// " col1 col2 "
std::string MakeHeaderRow(const Row& header_row) const;
@@ -118,21 +63,17 @@ class TablePrinterImpl {
std::string MakeTopSeparatorRow() const;

const std::vector<size_t>& column_sizes_;
const TablePrinter::Color text_color_;
const TablePrinter::Color table_color_;
};

std::string TablePrinterImpl::ToString(const Table& rows,
TablePrinter::Color text_color,
TablePrinter::Color table_color) {
std::string TablePrinterImpl::ToString(const Table& rows) {
if (!CheckNotEmptyAndAllColumnsTheSame(rows)) {
std::string error_msg = "ERROR - NOT ALL COLUMNS ARE THE SAME";
DCHECK(false) << error_msg;
return error_msg;
}

std::vector<size_t> column_sizes = MakeColumnSizeVector(rows);
TablePrinterImpl printer(column_sizes, text_color, table_color);
TablePrinterImpl printer(column_sizes);

std::stringstream output_ss;
output_ss << printer.MakeTopSeparatorRow() << "\n";
@@ -202,12 +143,8 @@ size_t TablePrinterImpl::ColumnPrintSize(const Table& table, size_t which_col) {
return max_col_size + 2; // +2 for padding on either side.
}

TablePrinterImpl::TablePrinterImpl(const std::vector<size_t>& column_sizes,
TablePrinter::Color text_color,
TablePrinter::Color table_printer)
: column_sizes_(column_sizes),
text_color_(text_color),
table_color_(table_printer) {}
TablePrinterImpl::TablePrinterImpl(const std::vector<size_t>& column_sizes)
: column_sizes_(column_sizes) {}

std::string TablePrinterImpl::MakeHeaderRow(const Row& header_row) const {
std::stringstream ss;
@@ -244,10 +181,10 @@ std::string TablePrinterImpl::MakeDataRow(const Row& row) const {
};

std::stringstream output_ss;
const std::string vertical_bar = AddColor(table_color_, "|");
const std::string vertical_bar = "|";

for (size_t i = 0; i < row.size(); ++i) {
std::string token = AddColor(text_color_, row[i]);
std::string token = row[i];

std::stringstream row_ss;
const int col_size = static_cast<int>(column_sizes_[i]);
@@ -276,7 +213,7 @@ std::string TablePrinterImpl::MakeRowDelimiter() const {
}
ss << "|";

std::string output = AddColor(table_color_, ss.str());
std::string output = ss.str();
return output;
}

@@ -294,21 +231,19 @@ std::string TablePrinterImpl::MakeTopSeparatorRow() const {
}
}
ss << " ";
std::string output = AddColor(table_color_, ss.str());
std::string output = ss.str();
return output;
}
} // namespace.

bool TablePrinter::SystemSupportsColor() { return SbLogIsTty(); }

TablePrinter::TablePrinter() : text_color_(kDefault), table_color_(kDefault) {}
TablePrinter::TablePrinter() {}

void TablePrinter::AddRow(const TablePrinter::Row& row) {
table_.push_back(row);
}

std::string TablePrinter::ToString() const {
return TablePrinterImpl::ToString(table_, text_color_, table_color_);
return TablePrinterImpl::ToString(table_);
}

} // namespace memory_settings
13 changes: 0 additions & 13 deletions cobalt/browser/memory_settings/table_printer.h
Original file line number Diff line number Diff line change
@@ -39,27 +39,14 @@ namespace memory_settings {
// "+--------+--------+\n";
class TablePrinter {
public:
static bool SystemSupportsColor();

enum Color { kDefault, kRed, kGreen, kYellow, kBlue, kMagenta, kCyan };

typedef std::vector<std::string> Row;
TablePrinter();

void AddRow(const Row& row);
std::string ToString() const;

// Adds color to the string output. kDefault will omit any color value and
// just do straight text.
// Note: If SbLogIsTty() returns false then color output is disabled and its
// recommended that these functions are not called.
void set_text_color(Color text_color) { text_color_ = text_color; }
void set_table_color(Color table_color) { table_color_ = table_color; }

private:
std::vector<Row> table_;
Color text_color_;
Color table_color_;
};

} // namespace memory_settings
17 changes: 0 additions & 17 deletions cobalt/browser/memory_settings/table_printer_test.cc
Original file line number Diff line number Diff line change
@@ -66,23 +66,6 @@ TEST(TablePrinter, ToString) {
table.ToString(), {"col1", "col2", "\n", "value1", "value2", "\n"}));
}

TEST(TablePrinter, ToStringWithColor) {
TablePrinter table;

// Adds color to table.
table.set_text_color(TablePrinter::kRed);
table.set_table_color(TablePrinter::kGreen);

// Add header.
table.AddRow(MakeRow2("col1", "col2"));
table.AddRow(MakeRow2("value1", "value2"));

EXPECT_TRUE(HasTokensInOrder(
table.ToString(),
{"col1", "col2", "\n", RED_START, "value1", COLOR_END, RED_START,
"value2", COLOR_END, "\n", GREEN_START, COLOR_END}));
}

#undef RED_START
#undef GREEN_START
#undef COLOR_END
3 changes: 3 additions & 0 deletions starboard/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -9,6 +9,9 @@ since the version previous to it.

## Version 16

## Deprecated `SbLogIsTty`
Removed, will use `isatty(fd)` if needed.

### Added Socket Waiter API for POSIX.
Introduced Starboard functions SbPosixSocketWaiterAdd, SbPosixSocketWaiterRemove
and callback function SbPosixSocketWaiterCallback to support POSIX based socket
1 change: 0 additions & 1 deletion starboard/android/shared/BUILD.gn
Original file line number Diff line number Diff line change
@@ -348,7 +348,6 @@ static_library("starboard_platform") {
"log_format.cc",
"log_internal.cc",
"log_internal.h",
"log_is_tty.cc",
"log_raw.cc",
"main.cc",
"max_output_buffers_lookup_table.cc",
21 changes: 0 additions & 21 deletions starboard/android/shared/log_is_tty.cc

This file was deleted.

2 changes: 2 additions & 0 deletions starboard/elf_loader/exported_symbols.cc
Original file line number Diff line number Diff line change
@@ -197,7 +197,9 @@ ExportedSymbols::ExportedSymbols() {
REGISTER_SYMBOL(SbLog);
REGISTER_SYMBOL(SbLogFlush);
REGISTER_SYMBOL(SbLogFormat);
#if SB_API_VERSION < 16
REGISTER_SYMBOL(SbLogIsTty);
#endif
REGISTER_SYMBOL(SbLogRaw);
REGISTER_SYMBOL(SbLogRawDumpStack);
REGISTER_SYMBOL(SbLogRawFormat);
Loading

0 comments on commit 8340a97

Please sign in to comment.