HTMLParser: Add UMA for main document tokenization time [chromium/src : main]

0 views
Skip to first unread message

AI Code Reviewer (Gerrit)

unread,
Sep 18, 2025, 2:22:41β€―AMΒ (3 days ago)Β Sep 18
to Yoshisato Yanagisawa, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org

AI Code Reviewer added 1 comment

File third_party/blink/renderer/core/html/parser/html_document_parser.h
Line 333, Patchset 1 (Latest): bool has_recorded_tokenization_time_ = false;
AI Code Reviewer . unresolved

nit: Blink style prefers to precede boolean member variables with verbs like 'is' or 'did'. For consistency with other booleans in this class (e.g., `did_pump_tokenizer_`), please consider renaming this to `is_tokenization_time_recorded_` or `did_record_tokenization_time_`. (Blink Style Guide: Naming - Precede boolean values with words like β€œis” and β€œdid”)

_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reason

This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent).AI reviews can sometimes be inaccurate; We appreciate your πŸ™ feedback πŸ™ to help us improve.[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ifd4558aefa1df136b48d3069550075678e2a0a4c
Gerrit-Change-Number: 6964134
Gerrit-PatchSet: 1
Gerrit-Owner: Yoshisato Yanagisawa <yyana...@chromium.org>
Gerrit-Reviewer: Yoshisato Yanagisawa <yyana...@chromium.org>
Gerrit-CC: AI Code Reviewer <peep-gen...@system.gserviceaccount.com>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-Comment-Date: Thu, 18 Sep 2025 06:22:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Yoshisato Yanagisawa (Gerrit)

unread,
Sep 18, 2025, 2:34:57β€―AMΒ (3 days ago)Β Sep 18
to AI Code Reviewer, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org

Yoshisato Yanagisawa added 1 comment

File third_party/blink/renderer/core/html/parser/html_document_parser.h
Line 333, Patchset 1: bool has_recorded_tokenization_time_ = false;
AI Code Reviewer . resolved

nit: Blink style prefers to precede boolean member variables with verbs like 'is' or 'did'. For consistency with other booleans in this class (e.g., `did_pump_tokenizer_`), please consider renaming this to `is_tokenization_time_recorded_` or `did_record_tokenization_time_`. (Blink Style Guide: Naming - Precede boolean values with words like β€œis” and β€œdid”)

_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reason

This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent).AI reviews can sometimes be inaccurate; We appreciate your πŸ™ feedback πŸ™ to help us improve.[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_

Yoshisato Yanagisawa

Done

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ifd4558aefa1df136b48d3069550075678e2a0a4c
Gerrit-Change-Number: 6964134
Gerrit-PatchSet: 2
Gerrit-Owner: Yoshisato Yanagisawa <yyana...@chromium.org>
Gerrit-Reviewer: Yoshisato Yanagisawa <yyana...@chromium.org>
Gerrit-CC: AI Code Reviewer <peep-gen...@system.gserviceaccount.com>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-Comment-Date: Thu, 18 Sep 2025 06:34:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: AI Code Reviewer <peep-gen...@system.gserviceaccount.com>
satisfied_requirement
unsatisfied_requirement
open
diffy

Kouhei Ueno (Gerrit)

unread,
Sep 18, 2025, 10:59:58β€―PMΒ (2 days ago)Β Sep 18
to Yoshisato Yanagisawa, Mason Freed, Jiacheng Guo, AI Code Reviewer, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org
Attention needed from Jiacheng Guo, Mason Freed and Yoshisato Yanagisawa

Kouhei Ueno voted and added 1 comment

Votes added by Kouhei Ueno

Code-Review+1

1 comment

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Kouhei Ueno . resolved

Mason: Would you mind taking a look?

I'm a bit worried about doing this in the hot path. Would you watch out for the speedometer metrics after landing this?

Open in Gerrit

Related details

Attention is currently required from:
  • Jiacheng Guo
  • Mason Freed
  • Yoshisato Yanagisawa
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Ifd4558aefa1df136b48d3069550075678e2a0a4c
    Gerrit-Change-Number: 6964134
    Gerrit-PatchSet: 2
    Gerrit-Owner: Yoshisato Yanagisawa <yyana...@chromium.org>
    Gerrit-Reviewer: Jiacheng Guo <g...@google.com>
    Gerrit-Reviewer: Kouhei Ueno <kou...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Yoshisato Yanagisawa <yyana...@chromium.org>
    Gerrit-CC: AI Code Reviewer <peep-gen...@system.gserviceaccount.com>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-Attention: Mason Freed <mas...@chromium.org>
    Gerrit-Attention: Jiacheng Guo <g...@google.com>
    Gerrit-Attention: Yoshisato Yanagisawa <yyana...@chromium.org>
    Gerrit-Comment-Date: Fri, 19 Sep 2025 02:59:52 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mason Freed (Gerrit)

    unread,
    Sep 19, 2025, 7:11:59β€―PMΒ (21 hours ago)Β Sep 19
    to Yoshisato Yanagisawa, Kouhei Ueno, Jiacheng Guo, AI Code Reviewer, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org
    Attention needed from Jiacheng Guo and Yoshisato Yanagisawa

    Mason Freed voted and added 4 comments

    Votes added by Mason Freed

    Code-Review+1

    4 comments

    Patchset-level comments
    Mason Freed . resolved

    LGTM with some small suggestions and questions.

    File third_party/blink/renderer/core/html/parser/html_document_parser.cc
    Line 773, Patchset 2 (Latest): base::ElapsedTimer timer;
    Mason Freed . unresolved

    Does this timer source have the resolution needed to measure very short calls like `NextToken` accurately?

    File tools/metrics/histograms/metadata/blink/histograms.xml
    Line 3003, Patchset 2 (Latest):<histogram name="Blink.HTMLParsing.TokenizationTime.MainDocument" units="ms"
    Mason Freed . unresolved

    This is a total time - would it be worth measuring the tokenization time as a percentage of the total parsing time, also?

    Line 3008, Patchset 2 (Latest): The total wall time spent in the tokenization phase for the main document's
    HTML. Tokenization is the process of converting the raw HTML text into a
    Mason Freed . unresolved

    This measures not just main document HTML though, right? It also measures tokenization from synchronous parsing like `innerHTML`, in the case that the fast-path html parser rejects the HTML. It's likely worth mentioning that. Perhaps better is just "all tokenization done by the HTMLDocumentParser" or something like that?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Jiacheng Guo
    • Yoshisato Yanagisawa
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: Ifd4558aefa1df136b48d3069550075678e2a0a4c
      Gerrit-Change-Number: 6964134
      Gerrit-PatchSet: 2
      Gerrit-Owner: Yoshisato Yanagisawa <yyana...@chromium.org>
      Gerrit-Reviewer: Jiacheng Guo <g...@google.com>
      Gerrit-Reviewer: Kouhei Ueno <kou...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Yoshisato Yanagisawa <yyana...@chromium.org>
      Gerrit-CC: AI Code Reviewer <peep-gen...@system.gserviceaccount.com>
      Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
      Gerrit-Attention: Jiacheng Guo <g...@google.com>
      Gerrit-Attention: Yoshisato Yanagisawa <yyana...@chromium.org>
      Gerrit-Comment-Date: Fri, 19 Sep 2025 23:11:47 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages