Skip to content

Conversation

@Ashod
Copy link
Contributor

@Ashod Ashod commented Jan 5, 2026

  • wsd: use std::array for logging
  • wsd: string_view in Log::prefix()
  • wsd: add new log prefixing implementation
  • wsd: use the new Prefix class
  • wsd: prime convertChronoClock
  • wsd: split time and clock tests to LoggingWhiteBoxTests.cpp
  • wsd: test: add new log Prefix tests
  • wsd: move the old Log::prefix to tests only

@github-project-automation github-project-automation bot moved this to To Review in Collabora Online Jan 5, 2026
@Ashod Ashod changed the title private/ash/prefix Log Prefix Jan 5, 2026
@Ashod Ashod requested a review from caolanm January 5, 2026 13:56
pos += 3;
pos = strcopy(level, pos);
memcpy(pos, level.data(), level.size());
pos += 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be pos += level.size() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's good observation. Unfortunately, the original code made the (reasonable) assumption that we only ever have 3-lettered levels. So that's been there all along.

Since changing this would be costly (in run-time performance), and since it wouldn't provide any benefits, I added an assertion to catch any changes that might break this assumption.

Incidentally, we have 2 spaces after the level, so adding a level with 4 characters wouldn't mess the log output at all. So there is a (slight) safety margin there, even without the assertion.

Ashod added 8 commits January 6, 2026 20:01
Change-Id: I96e866386b347a68fdd575ed90f40c4950ce75c2
Signed-off-by: Ashod Nakashian <[email protected]>
Change-Id: I7ff1d5c796ff4d8efa8bf84bbb9245ea708afa7f
Signed-off-by: Ashod Nakashian <[email protected]>
This implementation encapsulates the prefix
logic better and supports incremental
updates, which are anywhere between 20% and
30% faster than the naive implementation.

It also has a cleaner interface.

Change-Id: Id838c79436ea8a0f251fbf1f4eb972dc82425aad
Signed-off-by: Ashod Nakashian <[email protected]>
Change-Id: I1e31eeb2db282fd64ce316eb732a6a899e616136
Signed-off-by: Ashod Nakashian <[email protected]>
This slightly reduces the random noise
of the conversion.

Change-Id: I4942e4fd555ecf68d66387c704dd85c82d3d8e5c
Signed-off-by: Ashod Nakashian <[email protected]>
Change-Id: I0b097f3126589dcefa6b256b659ea53db03d9172
Signed-off-by: Ashod Nakashian <[email protected]>
Change-Id: Ic0641207717200a6334807d3ec949263a452f69f
Signed-off-by: Ashod Nakashian <[email protected]>
Change-Id: Ia5127c9f1869e9a57c6ed2da1231e757b6211303
Signed-off-by: Ashod Nakashian <[email protected]>
@Ashod Ashod force-pushed the private/ash/prefix branch from d5dabf6 to 3b7e84e Compare January 7, 2026 01:13
@caolanm
Copy link
Contributor

caolanm commented Jan 7, 2026

COOL 25.04 unit tests w/o cypress failure looks legit:

[ coolwsd ] ERR Failed to load unit-test lib /home/collabora/jenkins/workspace/github_online_master_debug_vs_co-25.04/test/../test/.libs/unit-base.so: undefined symbol: _ZN3Log15prefixReferenceERKNSt6chrono10time_pointINS0_3_V212system_clockENS0_8durationIlSt5ratioILl1ELl1000000000EEEEEEPcSt17basic_string_viewIcSt11char_traitsIcEE| common/Unit-server.cpp:27
Failed to initialize COOLWSD: Failed to load wsd unit test library.

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

Labels

None yet

Projects

Status: To Review

Development

Successfully merging this pull request may close these issues.

2 participants