Skip to content

Conversation

@thypon
Copy link
Member

@thypon thypon commented Nov 4, 2025

Supersedes: #855

@thypon thypon requested a review from a team as a code owner November 4, 2025 14:30
void BadExamples() {
// SHOULD TRIGGER: std::map for small/simple usage (consider base::flat_map)
// ruleid: chromium-std-map-performance-consideration
std::map<std::string, int> small_cache;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reported by reviewdog 🐶
[opengrep] Consider using base::flat_map/base::flat_set instead of std::map/std::set
for small containers or write-once scenarios. Chromium guidelines suggest:
- Use base::flat_map for small collections (< ~100 elements)
- Use base::flat_map for write-once, read-many scenarios
- Only use std::map for large containers with frequent writes
- Use std::map if you need pointer stability


Source: https://github.com/brave/security-action/blob/main/assets/opengrep_rules/client/container-performance-choice.yaml


Cc @thypon @cdesouza-chromium


// SHOULD TRIGGER: std::set for small collections (consider base::flat_set)
// ruleid: chromium-std-map-performance-consideration
std::set<int> small_ids;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reported by reviewdog 🐶
[opengrep] Consider using base::flat_map/base::flat_set instead of std::map/std::set
for small containers or write-once scenarios. Chromium guidelines suggest:
- Use base::flat_map for small collections (< ~100 elements)
- Use base::flat_map for write-once, read-many scenarios
- Only use std::map for large containers with frequent writes
- Use std::map if you need pointer stability


Source: https://github.com/brave/security-action/blob/main/assets/opengrep_rules/client/container-performance-choice.yaml


Cc @thypon @cdesouza-chromium


// SHOULD TRIGGER: std::multimap usage (consider alternatives)
// ruleid: chromium-std-map-performance-consideration
std::multimap<std::string, std::string> headers;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reported by reviewdog 🐶
[opengrep] Consider using base::flat_map/base::flat_set instead of std::map/std::set
for small containers or write-once scenarios. Chromium guidelines suggest:
- Use base::flat_map for small collections (< ~100 elements)
- Use base::flat_map for write-once, read-many scenarios
- Only use std::map for large containers with frequent writes
- Use std::map if you need pointer stability


Source: https://github.com/brave/security-action/blob/main/assets/opengrep_rules/client/container-performance-choice.yaml


Cc @thypon @cdesouza-chromium


// SHOULD TRIGGER: std::multiset usage (consider alternatives)
// ruleid: chromium-std-map-performance-consideration
std::multiset<int> duplicate_values;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reported by reviewdog 🐶
[opengrep] Consider using base::flat_map/base::flat_set instead of std::map/std::set
for small containers or write-once scenarios. Chromium guidelines suggest:
- Use base::flat_map for small collections (< ~100 elements)
- Use base::flat_map for write-once, read-many scenarios
- Only use std::map for large containers with frequent writes
- Use std::map if you need pointer stability


Source: https://github.com/brave/security-action/blob/main/assets/opengrep_rules/client/container-performance-choice.yaml


Cc @thypon @cdesouza-chromium

private:
// SHOULD TRIGGER: No justification for std::map usage
// ruleid: chromium-std-map-performance-consideration
std::map<std::string, bool> feature_flags_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reported by reviewdog 🐶
[opengrep] Consider using base::flat_map/base::flat_set instead of std::map/std::set
for small containers or write-once scenarios. Chromium guidelines suggest:
- Use base::flat_map for small collections (< ~100 elements)
- Use base::flat_map for write-once, read-many scenarios
- Only use std::map for large containers with frequent writes
- Use std::map if you need pointer stability


Source: https://github.com/brave/security-action/blob/main/assets/opengrep_rules/client/container-performance-choice.yaml


Cc @thypon @cdesouza-chromium


// SHOULD TRIGGER: std::map not ideal for write-once scenario
// ruleid: chromium-std-map-performance-consideration
std::map<std::string, int> limits;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reported by reviewdog 🐶
[opengrep] Consider using base::flat_map/base::flat_set instead of std::map/std::set
for small containers or write-once scenarios. Chromium guidelines suggest:
- Use base::flat_map for small collections (< ~100 elements)
- Use base::flat_map for write-once, read-many scenarios
- Only use std::map for large containers with frequent writes
- Use std::map if you need pointer stability


Source: https://github.com/brave/security-action/blob/main/assets/opengrep_rules/client/container-performance-choice.yaml


Cc @thypon @cdesouza-chromium

@thypon thypon force-pushed the chromium/container-performance-choice branch 2 times, most recently from 8f0a86a to 84a24a8 Compare November 11, 2025 07:26
Suggest base::flat_map over std::map for better performance with small
datasets, following Chromium performance guidelines.
@thypon thypon force-pushed the chromium/container-performance-choice branch from 84a24a8 to 47a35f0 Compare November 11, 2025 08:03
@github-actions
Copy link
Contributor

Opengrep Findings

📈 Comparison Results

  • Base branch findings: 0
  • Current branch findings: 129
  • Net change: +129 (100%)
  • New findings from rule changes: 129
  • New rules introduced: 1

Summary by Rule

Rule ID Findings Severity Change
client.chromium-std-map-performance-consideration 129 INFO 🆕 New

Detailed Findings

client.chromium-std-map-performance-consideration (129 findings)

  • browser/brave_ads/tabs/ads_tab_helper.h
  • browser/brave_rewards/android/brave_rewards_native_worker.h
  • browser/brave_shields/brave_shields_tab_helper.h
  • browser/brave_shields/brave_shields_web_contents_observer.h
  • browser/brave_wallet/brave_wallet_ethereum_chain_browsertest.cc
  • browser/brave_wallet/eth_pending_tx_tracker_unittest.cc
  • browser/net/brave_network_delegate_hsts_fingerprinting_browsertest.cc
  • browser/net/brave_request_handler.h
  • browser/net/url_context.h
  • browser/ui/views/brave_ads/notification_ad_popup_collection.cc

... and 93 more files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants