Closed Bug 1674045 Opened 1 year ago Closed 1 year ago

Deduplicate UniqueJSONStrings and avoid raw string pointers

Categories

(Core :: Gecko Profiler, task, P2)

task

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox84 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

UniqueJSONStrings is needed to write some of the new marker types, so it should be moved to a common location.
This also helps with the Base/Gecko Profilers de-duplication.

While working on it, raw pointer string arguments can be changed to spans, consistent with JSONWriter changes in bug 1657033.

The two identical copies are UniqueJSONStrings are combined and moved almost verbatim to BaseProfilerJSONWriter.h.

Document the class and methods.
GetOrAddIndex is only used internally, so it can be private.
SpliceStringTableElements can now only work on rvalue UniqueJSONStrings, this emphasizes that it shouldn't be used anymore after this call.

Depends on D95112

For consistency with JSONWriter (which UniqueJSONStrings' functions use), and for added safety and some efficiency, UniqueJSONStrings now takes Span<const char> arguments instead of raw pointers to null-terminated strings.

Depends on D95113

The previous patch removed the only two uses of ProfilerStringView::String().
Since it can be potentially expensive (creating a std::string object, sometimes allocating a buffer, and copying the string contents), it's best to remove it completely.

Blocks: 1674737
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/946e8a1c0924
Deduplicate UniqueJSONStrings - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/3489a01f7393
Add documentation to UniqueJSONStrings, and clean up - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/1d6cd5f0d26e
UniqueJSONStrings takes string spans instead of pointers - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/8e7a500b4ab9
Remove unused ProfilerStringView::String() - r=gregtatum
You need to log in before you can comment on or make changes to this bug.