[nfc] Clean up string allocations

Author: fhanau

src/workerd/api/actor-state.c++

@@ -929,7 +929,7 @@ class FacetOutgoingFactory final: public Fetcher::OutgoingFactory {
     return context.getMetrics().wrapActorSubrequestClient(context.getSubrequest(
         [&](TraceContext& tracing, IoChannelFactory& ioChannelFactory) {
       if (tracing.span.isObserved()) {
-        tracing.span.setTag("facet_name"_kjc, kj::str(name));
+        tracing.span.setTag("facet_name"_kjc, name.asPtr());
       }
 
       // Lazily initialize actorChannel

src/workerd/api/actor.c++

@@ -22,7 +22,7 @@ kj::Own<WorkerInterface> LocalActorOutgoingFactory::newSingleUseClient(
   return context.getMetrics().wrapActorSubrequestClient(context.getSubrequest(
       [&](TraceContext& tracing, IoChannelFactory& ioChannelFactory) {
     if (tracing.span.isObserved()) {
-      tracing.span.setTag("actor_id"_kjc, kj::str(actorId));
+      tracing.span.setTag("actor_id"_kjc, actorId.asPtr());
     }
 
     // Lazily initialize actorChannel
@@ -77,7 +77,7 @@ kj::Own<WorkerInterface> ReplicaActorOutgoingFactory::newSingleUseClient(
   return context.getMetrics().wrapActorSubrequestClient(context.getSubrequest(
       [&](TraceContext& tracing, IoChannelFactory& ioChannelFactory) {
     if (tracing.span.isObserved()) {
-      tracing.span.setTag("actor_id"_kjc, kj::heapString(actorId));
+      tracing.span.setTag("actor_id"_kjc, actorId.asPtr());
     }
 
     // Unlike in `GlobalActorOutgoingFactory`, we do not create this lazily, since our channel was

src/workerd/api/cache.c++

@@ -537,44 +537,42 @@ kj::Own<kj::HttpClient> Cache::getHttpClient(IoContext& context,
   TraceContext traceContext(
       context.makeTraceSpan(operationName), context.makeUserTraceSpan(operationName));
 
-  traceContext.setTag("url.full"_kjc, kj::str(url));
+  traceContext.setTag("url.full"_kjc, url);
   // TODO(o11y): Can we parse cacheControl more cleanly? For example, if tags are duplicated in the
   // same category we should choose the last value. We should also support these tags for
   // cache_match (where we should pull them from the returned response and need to keep the span
   // alive until then).
   KJ_IF_SOME(c, cacheControl) {
     // cacheability
     if (c.contains("no-store")) {
-      traceContext.setTag("cache_control.cacheability"_kjc, kj::str("no-store"));
+      traceContext.setTag("cache_control.cacheability"_kjc, "no-store"_kjc);
     } else if (c.contains("private")) {
-      traceContext.setTag("cache_control.cacheability"_kjc, kj::str("private"));
+      traceContext.setTag("cache_control.cacheability"_kjc, "private"_kjc);
     } else if (c.contains("public")) {
-      traceContext.setTag("cache_control.cacheability"_kjc, kj::str("public"));
+      traceContext.setTag("cache_control.cacheability"_kjc, "public"_kjc);
     }
 
     // expiration
     if (c.contains("no-cache")) {
-      traceContext.setTag("cache_control.expiration"_kjc, kj::str("no-cache"));
+      traceContext.setTag("cache_control.expiration"_kjc, "no-cache"_kjc);
     } else KJ_IF_SOME(idx, c.find("max-age="_kj)) {
       auto maybeNum = c.slice(idx + "max-age="_kj.size()).tryParseAs<double>();
       KJ_IF_SOME(num, maybeNum) {
-        traceContext.setTag(
-            "cache_control.expiration"_kjc, kj::str(kj::str("max-age="), kj::str(num)));
+        traceContext.setTag("cache_control.expiration"_kjc, kj::str("max-age="_kjc, num));
       }
     } else KJ_IF_SOME(idx, c.find("s-maxage="_kj)) {
       auto maybeNum = c.slice(idx + "s-maxage="_kj.size()).tryParseAs<double>();
       KJ_IF_SOME(num, maybeNum) {
-        traceContext.setTag(
-            "cache_control.expiration"_kjc, kj::str(kj::str("s-maxage="), kj::str(num)));
+        traceContext.setTag("cache_control.expiration"_kjc, kj::str("s-maxage="_kjc, num));
       }
     }
 
     // revalidation. Note: There are also stale-while-revalidate and stale-if-error directives, but
     // they are ignored by the Workers Cache API and we do not set them as tags accordingly.
     if (c.contains("must-revalidate")) {
-      traceContext.setTag("cache_control.revalidation"_kjc, kj::str("must-revalidate"));
+      traceContext.setTag("cache_control.revalidation"_kjc, "must-revalidate"_kjc);
     } else if (c.contains("proxy-revalidate")) {
-      traceContext.setTag("cache_control.revalidation"_kjc, kj::str("proxy-revalidate"));
+      traceContext.setTag("cache_control.revalidation"_kjc, "proxy-revalidate"_kjc);
     }
   }
   auto cacheClient = context.getCacheClient();

src/workerd/api/eventsource.c++

@@ -307,7 +307,7 @@ void EventSource::notifyMessages(jsg::Lock& js, kj::Array<PendingMessage> messag
     for (auto& message: messages) {
       auto data = kj::str(kj::delimited(kj::mv(message.data), "\n"_kjc));
       if (data.size() == 0) continue;
-      kj::String type = kj::mv(message.event).orDefault(kj::str("message"));
+      kj::String type = kj::mv(message.event).orDefault([]() { return kj::str("message"); });
       dispatchEventImpl(js,
           js.alloc<MessageEvent>(js, kj::mv(type), js.str(data), kj::mv(message.id),
               kj::none /** source **/, impl.map([](FetchImpl& i) -> jsg::Url& { return i.url; })));

src/workerd/api/kv.c++

@@ -266,10 +266,10 @@ kj::OneOf<jsg::Promise<KvNamespace::GetResult>, jsg::Promise<jsg::JsRef<jsg::JsM
   auto traceSpan = context.makeTraceSpan("kv_get"_kjc);
   auto userSpan = context.makeUserTraceSpan("kv_get"_kjc);
   TraceContext traceContext(kj::mv(traceSpan), kj::mv(userSpan));
-  traceContext.userSpan.setTag("db.system.name"_kjc, kj::str("cloudflare-kv"_kjc));
-  traceContext.userSpan.setTag("db.operation.name"_kjc, kj::str("get"_kjc));
-  traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, kj::str(bindingName));
-  traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, kj::str("KV"_kjc));
+  traceContext.userSpan.setTag("db.system.name"_kjc, "cloudflare-kv"_kjc);
+  traceContext.userSpan.setTag("db.operation.name"_kjc, "get"_kjc);
+  traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, bindingName.asPtr());
+  traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, "KV"_kjc);
 
   KJ_SWITCH_ONEOF(name) {
     KJ_CASE_ONEOF(arr, kj::Array<kj::String>) {
@@ -303,10 +303,10 @@ KvNamespace::getWithMetadata(jsg::Lock& js,
   auto traceSpan = context.makeTraceSpan("kv_getWithMetadata"_kjc);
   auto userSpan = context.makeUserTraceSpan("kv_getWithMetadata"_kjc);
   TraceContext traceContext(kj::mv(traceSpan), kj::mv(userSpan));
-  traceContext.userSpan.setTag("db.system.name"_kjc, kj::str("cloudflare-kv"_kjc));
-  traceContext.userSpan.setTag("db.operation.name"_kjc, kj::str("get"_kjc));
-  traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, kj::str(bindingName));
-  traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, kj::str("KV"_kjc));
+  traceContext.userSpan.setTag("db.system.name"_kjc, "cloudflare-kv"_kjc);
+  traceContext.userSpan.setTag("db.operation.name"_kjc, "get"_kjc);
+  traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, bindingName.asPtr());
+  traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, "KV"_kjc);
   KJ_SWITCH_ONEOF(name) {
     KJ_CASE_ONEOF(arr, kj::Array<kj::String>) {
       return context.attachSpans(js,
@@ -330,7 +330,7 @@ jsg::Promise<KvNamespace::GetWithMetadataResult> KvNamespace::getWithMetadataImp
     LimitEnforcer::KvOpType op) {
   validateKeyName("GET", name);
 
-  traceContext.userSpan.setTag("cloudflare.kv.query.keys"_kjc, kj::str(name));
+  traceContext.userSpan.setTag("cloudflare.kv.query.keys"_kjc, name.asPtr());
   traceContext.userSpan.setTag("cloudflare.kv.query.keys.count"_kjc, static_cast<int64_t>(1));
 
   kj::Url url;
@@ -372,7 +372,7 @@ jsg::Promise<KvNamespace::GetWithMetadataResult> KvNamespace::getWithMetadataImp
       -> jsg::Promise<KvNamespace::GetWithMetadataResult> {
     auto cacheStatus =
         response.headers->get(context.getHeaderIds().cfCacheStatus).map([&](kj::StringPtr cs) {
-      traceContext.userSpan.setTag("cloudflare.kv.response.cache_status"_kjc, kj::str(cs));
+      traceContext.userSpan.setTag("cloudflare.kv.response.cache_status"_kjc, cs);
       return jsg::JsRef<jsg::JsValue>(js, js.strIntern(cs));
     });
 
@@ -463,10 +463,10 @@ jsg::Promise<jsg::JsRef<jsg::JsValue>> KvNamespace::list(
     auto userSpan = context.makeUserTraceSpan("kv_list"_kjc);
     TraceContext traceContext(kj::mv(traceSpan), kj::mv(userSpan));
 
-    traceContext.userSpan.setTag("db.system.name"_kjc, kj::str("cloudflare-kv"_kjc));
-    traceContext.userSpan.setTag("db.operation.name"_kjc, kj::str("list"_kjc));
-    traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, kj::str(bindingName));
-    traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, kj::str("KV"_kjc));
+    traceContext.userSpan.setTag("db.system.name"_kjc, "cloudflare-kv"_kjc);
+    traceContext.userSpan.setTag("db.operation.name"_kjc, "list"_kjc);
+    traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, bindingName.asPtr());
+    traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, "KV"_kjc);
 
     kj::Url url;
     url.scheme = kj::str("https");
@@ -480,13 +480,13 @@ jsg::Promise<jsg::JsRef<jsg::JsValue>> KvNamespace::list(
       }
       KJ_IF_SOME(maybePrefix, o.prefix) {
         KJ_IF_SOME(prefix, maybePrefix) {
-          traceContext.userSpan.setTag("cloudflare.kv.query.prefix"_kjc, kj::str(prefix));
+          traceContext.userSpan.setTag("cloudflare.kv.query.prefix"_kjc, prefix.asPtr());
           url.query.add(kj::Url::QueryParam{kj::str("prefix"), kj::str(prefix)});
         }
       }
       KJ_IF_SOME(maybeCursor, o.cursor) {
         KJ_IF_SOME(cursor, maybeCursor) {
-          traceContext.userSpan.setTag("cloudflare.kv.query.cursor"_kjc, kj::str(cursor));
+          traceContext.userSpan.setTag("cloudflare.kv.query.cursor"_kjc, cursor.asPtr());
           url.query.add(kj::Url::QueryParam{kj::str("cursor"), kj::str(cursor)});
         }
       }
@@ -552,11 +552,11 @@ jsg::Promise<void> KvNamespace::put(jsg::Lock& js,
     auto userSpan = context.makeUserTraceSpan("kv_put"_kjc);
     TraceContext traceContext(kj::mv(traceSpan), kj::mv(userSpan));
 
-    traceContext.userSpan.setTag("db.system.name"_kjc, kj::str("cloudflare-kv"_kjc));
-    traceContext.userSpan.setTag("db.operation.name"_kjc, kj::str("put"_kjc));
-    traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, kj::str(bindingName));
-    traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, kj::str("KV"_kjc));
-    traceContext.userSpan.setTag("cloudflare.kv.query.keys"_kjc, kj::str(name));
+    traceContext.userSpan.setTag("db.system.name"_kjc, "cloudflare-kv"_kjc);
+    traceContext.userSpan.setTag("db.operation.name"_kjc, "put"_kjc);
+    traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, bindingName.asPtr());
+    traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, "KV"_kjc);
+    traceContext.userSpan.setTag("cloudflare.kv.query.keys"_kjc, name.asPtr());
     traceContext.userSpan.setTag("cloudflare.kv.query.keys.count"_kjc, static_cast<int64_t>(1));
 
     kj::Url url;
@@ -612,16 +612,15 @@ jsg::Promise<void> KvNamespace::put(jsg::Lock& js,
       KJ_CASE_ONEOF(text, kj::String) {
         headers.setPtr(kj::HttpHeaderId::CONTENT_TYPE, MimeType::PLAINTEXT_STRING);
         expectedBodySize = static_cast<uint64_t>(text.size());
-        traceContext.userSpan.setTag("cloudflare.kv.query.value_type"_kjc, kj::str("text"));
+        traceContext.userSpan.setTag("cloudflare.kv.query.value_type"_kjc, "text"_kjc);
       }
       KJ_CASE_ONEOF(data, kj::Array<byte>) {
         expectedBodySize = static_cast<uint64_t>(data.size());
-        traceContext.userSpan.setTag("cloudflare.kv.query.value_type"_kjc, kj::str("ArrayBuffer"));
+        traceContext.userSpan.setTag("cloudflare.kv.query.value_type"_kjc, "ArrayBuffer"_kjc);
       }
       KJ_CASE_ONEOF(stream, jsg::Ref<ReadableStream>) {
         expectedBodySize = stream->tryGetLength(StreamEncoding::IDENTITY);
-        traceContext.userSpan.setTag(
-            "cloudflare.kv.query.value_type"_kjc, kj::str("ReadableStream"));
+        traceContext.userSpan.setTag("cloudflare.kv.query.value_type"_kjc, "ReadableStream"_kjc);
       }
     }
 
@@ -688,11 +687,11 @@ jsg::Promise<void> KvNamespace::delete_(jsg::Lock& js, kj::String name) {
     auto userSpan = context.makeUserTraceSpan("kv_delete"_kjc);
     TraceContext traceContext(kj::mv(traceSpan), kj::mv(userSpan));
 
-    traceContext.userSpan.setTag("db.system.name"_kjc, kj::str("cloudflare-kv"_kjc));
-    traceContext.userSpan.setTag("db.operation.name"_kjc, kj::str("delete"_kjc));
-    traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, kj::str(bindingName));
-    traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, kj::str("KV"_kjc));
-    traceContext.userSpan.setTag("cloudflare.kv.query.keys"_kjc, kj::str(name));
+    traceContext.userSpan.setTag("db.system.name"_kjc, "cloudflare-kv"_kjc);
+    traceContext.userSpan.setTag("db.operation.name"_kjc, "delete"_kjc);
+    traceContext.userSpan.setTag("cloudflare.binding.name"_kjc, bindingName.asPtr());
+    traceContext.userSpan.setTag("cloudflare.binding.type"_kjc, "KV"_kjc);
+    traceContext.userSpan.setTag("cloudflare.kv.query.keys"_kjc, name.asPtr());
     traceContext.userSpan.setTag("cloudflare.kv.query.keys.count"_kjc, static_cast<int64_t>(1));
 
     auto urlStr = kj::str("https://fake-host/", kj::encodeUriComponent(name), "?urlencoded=true");

src/workerd/api/memory-cache.c++

@@ -135,7 +135,7 @@ void SharedMemoryCache::putWhileLocked(ThreadUnsafeData& data,
   size_t valueSize = value->bytes.size();
 
   auto writeSpan = IoContext::current().makeTraceSpan("memory_cache_write"_kjc);
-  writeSpan.setTag("key"_kjc, kj::str(key));
+  writeSpan.setTag("key"_kjc, key.asPtr());
   writeSpan.setTag("value_size"_kjc, static_cast<double>(valueSize));
   writeSpan.setTag("has_expiration"_kjc, expiration != kj::none);
 

src/workerd/api/streams/internal.c++

@@ -25,7 +25,7 @@ namespace {
   if (IoContext::hasCurrent()) {
     auto& context = IoContext::current();
     if (context.isInspectorEnabled()) {
-      context.logWarning(kj::str(message));
+      context.logWarning(message);
     }
   }
 

src/workerd/api/worker-rpc.c++

@@ -703,12 +703,12 @@ JsRpcStub::~JsRpcStub() noexcept(false) {
     //   IoContext tear-down is problematic since logWarningOnce() is a method on
     //   IoContext...
     if (IoContext::hasCurrent()) {
-      IoContext::current().logWarningOnce(kj::str(
+      IoContext::current().logWarningOnce(
           "An RPC stub was not disposed properly. You must call dispose() on all stubs in order to "
           "let the other side know that you are no longer using them. You cannot rely on "
           "the garbage collector for this because it may take arbitrarily long before actually "
           "collecting unreachable objects. As a shortcut, calling dispose() on the result of "
-          "an RPC call disposes all stubs within it."));
+          "an RPC call disposes all stubs within it."_kj);
     }
   }
 }
@@ -738,11 +738,11 @@ RpcStubDisposalGroup::~RpcStubDisposalGroup() noexcept(false) {
       //
       // TODO(cleanup): Same comment as in ~JsRpcStub().
       if (IoContext::hasCurrent()) {
-        IoContext::current().logWarningOnce(kj::str(
+        IoContext::current().logWarningOnce(
             "An RPC result was not disposed properly. One of the RPC calls you made expects you "
             "to call dispose() on the return value, but you didn't do so. You cannot rely on "
             "the garbage collector for this because it may take arbitrarily long before actually "
-            "collecting unreachable objects."));
+            "collecting unreachable objects."_kj);
       }
     }
   } else {

src/workerd/io/io-own.c++

@@ -56,10 +56,8 @@ void DeleteQueue::checkFarGet(const DeleteQueue& deleteQueue, const std::type_in
 }
 
 void DeleteQueue::checkWeakGet(workerd::WeakRef<IoContext>& weak) {
-  if (!weak.isValid()) {
-    JSG_FAIL_REQUIRE(
-        Error, kj::str("Couldn't complete operation because the execution context has ended."));
-  }
+  JSG_REQUIRE(weak.isValid(), Error,
+      "Couldn't complete operation because the execution context has ended.");
 }
 
 kj::Promise<void> DeleteQueue::resetCrossThreadSignal() const {

src/workerd/io/trace.c++

@@ -929,7 +929,7 @@ Attribute Attribute::clone() const {
 }
 
 kj::String Attribute::toString() const {
-  return kj::str("Attribute: ", name, ", ", kj::str(value));
+  return kj::str("Attribute: ", name, ", ", value);
 }
 
 Return::Return(kj::Maybe<FetchResponseInfo> info): info(kj::mv(info)) {}