Skip to content

Commit

Permalink
Materialize the paged message before interacting with it.
Browse files Browse the repository at this point in the history
Previously the message would be materialized multiple times:
- `1` time in `addMsgCtx`
- `3` or `1 + 3p` times in `pagedLogging`
  - Lower Bound (single page): `3` times
    - `1` time to calculate the number of pages
    - `1` time in `addPageCtx` to calculate the page length
    - `1` time when calling `logOpWithCtx`
  - Upper Bound (`p` pages): `1 + 3p`
    - `1` time to calculate the number of pages
    - `1` time per page to split out each page
    - `1` time per page in `addPageCtx` to calculate the page length
    - `1` time per page when calling `logOpWithCtx`

Materializing early gives these improvements:
- Single page: from `4` times to `1` time
- Multiple pages: from `2 + 3p` times to `1` time

Also removes some by-name parameters that were always called with
materialized values.
  • Loading branch information
morgen-peschke committed Jun 14, 2024
1 parent 8bec275 commit 230e82d
Showing 1 changed file with 15 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ object PagingSelfAwareStructuredLogger {
private val pageSize = pageSizeK * 1024

private def pagedLogging(
logOpWithCtx: Map[String, String] => (=> String) => F[Unit],
logOpWithCtx: Map[String, String] => String => F[Unit],
ctx: Map[String, String],
logSplitId: String,
msg: => String
msg: String
): F[Unit] = {
val numOfPagesRaw = (msg.length - 1) / pageSize + 1
val numOfPages = Math.min(numOfPagesRaw, maxPageNeeded)
Expand Down Expand Up @@ -118,7 +118,7 @@ object PagingSelfAwareStructuredLogger {
}

private def addMsgCtx(
msg: => String,
msg: String,
ctx: Map[String, String]
): F[(String, Map[String, String])] =
randomUUID.map { uuid =>
Expand All @@ -136,9 +136,9 @@ object PagingSelfAwareStructuredLogger {
}

private def addPageCtx(
page: => String,
pageNum: => Int,
totalPages: => Int,
page: String,
pageNum: Int,
totalPages: Int,
ctx: Map[String, String]
): Map[String, String] =
ctx
Expand All @@ -148,13 +148,19 @@ object PagingSelfAwareStructuredLogger {

private def doLogging(
loggingLevelChk: => F[Boolean],
logOpWithCtx: Map[String, String] => (=> String) => F[Unit],
logOpWithCtx: Map[String, String] => String => F[Unit],
msg: => String,
ctx: Map[String, String] = Map()
): F[Unit] = {
loggingLevelChk.ifM(
addMsgCtx(msg, ctx).flatMap { case (logSplitId, newCtx) =>
pagedLogging(logOpWithCtx, newCtx, logSplitId, msg)
{
// At this point we know we're going to log and/or interact
// with msg, so we materialize the message here so we don't
// materialize it multiple times
val materializedMsg = msg
addMsgCtx(materializedMsg, ctx).flatMap { case (logSplitId, newCtx) =>
pagedLogging(logOpWithCtx, newCtx, logSplitId, materializedMsg)
}
},
Applicative[F].unit
)
Expand Down

0 comments on commit 230e82d

Please sign in to comment.