src: add fast api calls to push span strings #289
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis update refactors the way tracing span data is handled and recorded within the codebase. Instead of concatenating multiple pieces of information (such as trace IDs or HTTP request URLs) into single strings, the new approach separates these values and processes them as distinct components. This change is reflected across the JavaScript and C++ layers, introducing new methods and type definitions for handling multiple string arguments, updating property types, and modifying the way span IDs are managed and recorded. The control flow and error handling remain unchanged, with the primary focus on improved data handling and clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant ClientRequest
participant Span
participant BindingData
ClientRequest->>Span: _pushSpanDataString3(protocol, host, path)
Span->>BindingData: pushSpanDataString3(trace_id, type, protocol, host, path)
BindingData->>BindingData: PushSpanDataStringImpl3(...)
Note right of BindingData: Concatenate and store span data
sequenceDiagram
participant Tracer
participant Span
Tracer->>Span: new Span(internalId, parentSpanId, ...)
Span->>Span: _pushSpanDataString(type, value)
Span->>Span: _pushSpanDataString3(type, val1, val2, val3)
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/internal/otel/trace.js (1)
222-225: Add_pushSpanDataString3for handling three string arguments.
This enables more efficient pushing of multiple adjacent string values. As an optional improvement, consider basic argument validation or type checks to prevent future misuse, especially if external code references this method.src/nsolid/nsolid_api.cc (1)
2361-2410: Implement triple-string push methods with an assertion to restrict usage.
The assertion onSpan::kSpanHttpReqUrlensures this method is domain-specific. This is fine if it’s strictly used for HTTP request URLs. If you expand triple-string usage beyond URLs, consider removing or generalizing that assertion. Also review whether"//"is always safe as a separator.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
lib/_http_client.js(1 hunks)lib/_http_server.js(2 hunks)lib/internal/otel/trace.js(4 hunks)src/node_external_reference.h(2 hunks)src/nsolid/nsolid_api.cc(5 hunks)src/nsolid/nsolid_bindings.h(3 hunks)src/nsolid/nsolid_trace.cc(1 hunks)src/nsolid/nsolid_trace.h(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/nsolid/nsolid_api.cc (3)
src/nsolid/nsolid_bindings.h (21)
args(25-26)args(30-31)args(35-36)args(40-41)args(51-52)args(62-63)args(72-73)data(28-28)data(33-33)data(38-38)data(46-49)data(57-60)data(68-71)data(80-85)receiver(27-27)receiver(32-32)receiver(37-37)receiver(42-45)receiver(53-56)receiver(64-67)receiver(74-79)src/nsolid/nsolid_api.h (19)
isolate(250-250)isolate(255-255)isolate(308-308)isolate(309-309)isolate(315-318)isolate(319-322)isolate(556-558)isolate(559-560)isolate(617-621)isolate(892-894)isolate(892-892)data(75-75)data(76-76)data(500-503)data(505-508)data(510-513)data(514-517)data(524-527)context(251-251)src/nsolid/nsolid_trace.h (7)
type(135-135)val(149-149)val(225-227)val(225-225)createSpanProp(127-127)createSpanProp(217-220)createSpanProp(217-218)
src/node_external_reference.h (1)
src/nsolid/nsolid_bindings.h (7)
receiver(27-27)receiver(32-32)receiver(37-37)receiver(42-45)receiver(53-56)receiver(64-67)receiver(74-79)
🔇 Additional comments (18)
src/nsolid/nsolid_trace.h (1)
33-36: Improved tracing granularity with separate span ID properties.The changes to
NSOLID_SPAN_PROP_TYPES_STRINGSmacro replace the singlekSpanOtelIdswith separate properties for trace ID, span ID, parent span ID, and status message. This allows for more efficient handling of each component individually.lib/_http_client.js (1)
486-486: Performance optimization: Using fast API calls for URL parts.The implementation now uses
_pushSpanDataString3to pass individual URL components directly to C++ instead of concatenating them in JavaScript. This prevents the creation of non-flattened V8 strings, enabling the fast API path for better performance.lib/_http_server.js (2)
1114-1114: Standardized protocol format with trailing colon.The protocol string now includes a trailing colon (
'https:'instead of'https'), which is the standard format for URL protocols and ensures consistency across the codebase.
1128-1128: Performance optimization: Using fast API calls for URL components.Similar to the client-side changes, this implementation now uses
_pushSpanDataString3to pass individual URL components directly to C++ instead of concatenating strings in JavaScript, enabling the fast API path for better performance.src/nsolid/nsolid_trace.cc (1)
52-66: Refactored span ID handling for cleaner and more efficient implementation.The implementation now handles trace ID, span ID, and parent span ID as separate properties rather than parsing them from a combined string. This simplifies the code by:
- Eliminating string parsing logic
- Providing direct assignment to respective storage fields
- Creating a more maintainable structure with explicit breaks after each case
These changes align with the JavaScript-side optimizations for better tracing performance.
src/node_external_reference.h (2)
91-102: Introduce new function pointer types for single/triple string pushes.
These additions cleanly match the fast API calls described in your native binding code. They appear correctly typed, takingFastOneByteStringparameters and a V8 receiver.
133-134: Register new pointer types in ALLOWED_EXTERNAL_REFERENCE_TYPES.
IncludingCFunctionPushSpanDataStringandCFunctionPushSpanDataString3in the macro ensures they are properly recognized and managed as external references. This step looks correct and complete.lib/internal/otel/trace.js (3)
58-62: Push separate ID properties instead of a combined string.
Splitting outtraceId,spanId, and optionallyparentSpanIdclarifies the property values in each push call. This is easier to maintain and more explicit than a single combined ID string.
216-220: InvokepushSpanDataStringwith minimal overhead.
Callingbinding.pushSpanDataStringdirectly looks straightforward. Since these lines only run when the span is sampled, there's no extra overhead in unsampled scenarios.
280-280: PassparentContext?.spanIdas an explicit parameter.
ReferencingparentContext?.spanIddirectly here further clarifies parent-child relationships for child spans. This is a clear improvement from parsing or concatenating the parent ID.src/nsolid/nsolid_bindings.h (3)
7-7: Includev8-fast-api-calls.hfor fast string handling.
This new include is necessary forFastOneByteString, enabling more efficient string processing in the binding implementation.
62-86: Define new slow/fast push APIs for single and triple string arguments.
These function prototypes cleanly separate slow and fast variants for pushing string data into spans. Be mindful thatPushSpanDataStringImpl3might only apply to specific span property types, so consider clarifying or documenting its usage constraints more explicitly.
100-101: Addfast_push_span_data_string_andfast_push_span_data_string3_.
Declaring thesev8::CFunctionfields here keeps the APIs consistent with the rest of the binding code. Make sure both are registered in the same scope and used only in contexts expecting fast calls.src/nsolid/nsolid_api.cc (5)
42-42: UseFastOneByteStringfor improved string performance.
Thisusingstatement is essential to integrate V8’s fast API calls. Make sure fast strings remain valid for the duration of calls and that the isolate isn't accessed concurrently from multiple threads, preserving V8 usage constraints.
2322-2359: Implement Slow/Fast PushSpanDataString with consistent validation.
Your checks (DCHECK_EQ(args.Length(), 3)and type checks) prevent misuse in internal code. However, no fallback or user error handling is present if arguments are invalid. That’s generally acceptable for internal-only APIs, but keep it in mind for potential future expansions.
2963-2966: Register fast push methods for single/triple string data.
These lines correctly bind the static CFunctions to the fast-call entry points. This helps unify slow vs. fast code paths when bridging from JS to C++.
3006-3015: Add new fast binding methods in JS object exports.
CallingSetFastMethodfor “pushSpanDataString” and “pushSpanDataString3” ensures both are properly exposed to userland code. The naming convention is consistent, matching the slow methods.
3148-3155: Register the new string push references inRegisterExternalReferences.
Ensuring both slow and fast push references are recognized closes the loop for tracing external references. This is essential for external reference registration in Node.
santigimeno
force-pushed
the
santi/fast_push_string
branch
from
8af94e2 to
f996ee1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/nsolid/nsolid_api.cc (2)
2373-2381: Duplicate concern about validating 'type'.
This fast-path function uses the same signature and logic as the slow variant. The same comment regarding potential invalidtypeapplies here.
2418-2431: Duplicate concern about validating triple-string 'type'.
This fast method repeats the same approach for the triple-string scenario. The same recommendation applies regarding checking or handling an invalidtypeat runtime.
🧹 Nitpick comments (3)
src/nsolid/nsolid_api.cc (3)
2357-2371: Consider validating 'type' before casting.
The method relies on debug-only checks (DCHECK) and ultimately caststypetoSpan::PropType, but there's no runtime validation. If invalid values are passed in release builds, it could lead to undefined behavior.You could add a runtime check or gracefully handle out-of-range values, for example:
+ if (type > static_cast<uint32_t>(Span::kUnknownLast)) { + // Return or throw an error for invalid property type + return; + }
2396-2415: Tune the error handling for triple string arguments if 'type' is unexpected.
WhilePushSpanDataStringImpl3eventually includes an assertion forkSpanHttpReqUrl, consider adding a runtime check here if you want to catch mismatches and produce an explicit error message rather than only relying on the debug assertion deeper in the call stack.
2432-2445: Assertion is good, but consider a runtime check in release builds.
ASSERT_EQ(prop_type, Span::kSpanHttpReqUrl)confirms expectations in debug mode. A release-mode safeguard would ensure stability in production.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
lib/_http_client.js(1 hunks)lib/_http_server.js(2 hunks)lib/internal/otel/trace.js(4 hunks)src/node_external_reference.h(2 hunks)src/nsolid/nsolid_api.cc(5 hunks)src/nsolid/nsolid_bindings.h(3 hunks)src/nsolid/nsolid_trace.cc(1 hunks)src/nsolid/nsolid_trace.h(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- lib/_http_client.js
- src/nsolid/nsolid_trace.cc
- lib/_http_server.js
- src/nsolid/nsolid_trace.h
- lib/internal/otel/trace.js
- src/node_external_reference.h
- src/nsolid/nsolid_bindings.h
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint-js-and-md
- GitHub Check: coverage-windows
- GitHub Check: build-docs
🔇 Additional comments (4)
src/nsolid/nsolid_api.cc (4)
2383-2394: Looks good.
This implementation correctly constructs and pushes the string-based property. No further issues found.
2998-3001: Definition of fast function objects is consistent.
No issues found.
3041-3051: Binding setup looks correct.
Registering both slow and fast methods under the correct names is consistent with the existing pattern.
3183-3190: Registration of slow/fast string-based span data methods is consistent.
No concerns here. This aligns perfectly with how other fast API calls are registered.
santigimeno
force-pushed
the
santi/fast_push_string
branch
from
f996ee1 to
4f9d18b
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
santigimeno
force-pushed
the
santi/fast_push_string
branch
2 times, most recently
from
275a699 to
2bbf7f2
Compare
santigimeno
force-pushed
the
santi/fast_push_string
branch
from
2bbf7f2 to
c6fb59e
Compare
This commit introduces performance optimizations for tracing span attribute handling by adding two new fast API calls: 1. FastPushSpanDataString - Optimized version of the existing string push method 2. FastPushSpanDataString3 - New method to efficiently handle three string values in a single call Key changes include: - Avoided JavaScript string concatenation which produces non-flattened V8 strings that cannot trigger fast API paths - Implemented direct passing of individual string components to C++ where concatenation can happen after the fast API boundary - Refactored HTTP URL construction in both client and server to leverage this optimization - Improved span context handling by storing trace ID, span ID, and parent span ID as separate attributes rather than a single concatenated string These optimizations significantly improve performance in high load scenarios: - Up to 3.24% higher throughput in maximum load tests - Up to 57.75% lower latency at P99.9 under constant high rate (50k req/sec) - Substantial improvements across all high percentile latencies (P95-P99.999) - Most dramatic gains observed in tail latencies under sustained heavy load === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 40k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 2.58 | 2.34 | N/A | | | P50 Latency (ms) | 1.09 | 1.10 | +0.80% | *** | | P90 Latency (ms) | 2.22 | 2.20 | -0.63% | *** | | P99 Latency (ms) | 3.09 | 2.99 | -3.31% | *** | | Avg Req/Sec | 39,992.50 | 39,992.53 | +0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 50k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 9.92 | 5.20 | N/A | | | P50 Latency (ms) | 0.95 | 0.94 | -0.54% | | | P90 Latency (ms) | 2.54 | 2.35 | -7.47% | *** | | P99 Latency (ms) | 12.00 | 5.58 | -53.49% | *** | | Avg Req/Sec | 49,990.78 | 49,990.72 | -0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk (maximum throughput) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Threads: 2 +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 0.65 | 0.64 | N/A | | | P50 Latency (ms) | 0.17 | 0.16 | -3.48% | *** | | P90 Latency (ms) | 0.33 | 0.32 | -2.30% | *** | | P99 Latency (ms) | 0.36 | 0.35 | -3.33% | *** | | Avg Req/Sec | 52,398.20 | 54,094.55 | +3.24% | *** | +------------------+-------------+-------------+------------+-----+
santigimeno
force-pushed
the
santi/fast_push_string
branch
from
c6fb59e to
ee72779
Compare
This commit introduces performance optimizations for tracing span attribute handling by adding two new fast API calls: 1. FastPushSpanDataString - Optimized version of the existing string push method 2. FastPushSpanDataString3 - New method to efficiently handle three string values in a single call Key changes include: - Avoided JavaScript string concatenation which produces non-flattened V8 strings that cannot trigger fast API paths - Implemented direct passing of individual string components to C++ where concatenation can happen after the fast API boundary - Refactored HTTP URL construction in both client and server to leverage this optimization - Improved span context handling by storing trace ID, span ID, and parent span ID as separate attributes rather than a single concatenated string These optimizations significantly improve performance in high load scenarios: - Up to 3.24% higher throughput in maximum load tests - Up to 57.75% lower latency at P99.9 under constant high rate (50k req/sec) - Substantial improvements across all high percentile latencies (P95-P99.999) - Most dramatic gains observed in tail latencies under sustained heavy load === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 40k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 2.58 | 2.34 | N/A | | | P50 Latency (ms) | 1.09 | 1.10 | +0.80% | *** | | P90 Latency (ms) | 2.22 | 2.20 | -0.63% | *** | | P99 Latency (ms) | 3.09 | 2.99 | -3.31% | *** | | Avg Req/Sec | 39,992.50 | 39,992.53 | +0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 50k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 9.92 | 5.20 | N/A | | | P50 Latency (ms) | 0.95 | 0.94 | -0.54% | | | P90 Latency (ms) | 2.54 | 2.35 | -7.47% | *** | | P99 Latency (ms) | 12.00 | 5.58 | -53.49% | *** | | Avg Req/Sec | 49,990.78 | 49,990.72 | -0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk (maximum throughput) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Threads: 2 +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 0.65 | 0.64 | N/A | | | P50 Latency (ms) | 0.17 | 0.16 | -3.48% | *** | | P90 Latency (ms) | 0.33 | 0.32 | -2.30% | *** | | P99 Latency (ms) | 0.36 | 0.35 | -3.33% | *** | | Avg Req/Sec | 52,398.20 | 54,094.55 | +3.24% | *** | +------------------+-------------+-------------+------------+-----+ PR-URL: #289 Reviewed-By: Juan José Arboleda <[email protected]>
|
Landed in 6ea71c8 |
This commit introduces performance optimizations for tracing span attribute handling by adding two new fast API calls: 1. FastPushSpanDataString - Optimized version of the existing string push method 2. FastPushSpanDataString3 - New method to efficiently handle three string values in a single call Key changes include: - Avoided JavaScript string concatenation which produces non-flattened V8 strings that cannot trigger fast API paths - Implemented direct passing of individual string components to C++ where concatenation can happen after the fast API boundary - Refactored HTTP URL construction in both client and server to leverage this optimization - Improved span context handling by storing trace ID, span ID, and parent span ID as separate attributes rather than a single concatenated string These optimizations significantly improve performance in high load scenarios: - Up to 3.24% higher throughput in maximum load tests - Up to 57.75% lower latency at P99.9 under constant high rate (50k req/sec) - Substantial improvements across all high percentile latencies (P95-P99.999) - Most dramatic gains observed in tail latencies under sustained heavy load === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 40k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 2.58 | 2.34 | N/A | | | P50 Latency (ms) | 1.09 | 1.10 | +0.80% | *** | | P90 Latency (ms) | 2.22 | 2.20 | -0.63% | *** | | P99 Latency (ms) | 3.09 | 2.99 | -3.31% | *** | | Avg Req/Sec | 39,992.50 | 39,992.53 | +0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 50k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 9.92 | 5.20 | N/A | | | P50 Latency (ms) | 0.95 | 0.94 | -0.54% | | | P90 Latency (ms) | 2.54 | 2.35 | -7.47% | *** | | P99 Latency (ms) | 12.00 | 5.58 | -53.49% | *** | | Avg Req/Sec | 49,990.78 | 49,990.72 | -0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk (maximum throughput) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Threads: 2 +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 0.65 | 0.64 | N/A | | | P50 Latency (ms) | 0.17 | 0.16 | -3.48% | *** | | P90 Latency (ms) | 0.33 | 0.32 | -2.30% | *** | | P99 Latency (ms) | 0.36 | 0.35 | -3.33% | *** | | Avg Req/Sec | 52,398.20 | 54,094.55 | +3.24% | *** | +------------------+-------------+-------------+------------+-----+ PR-URL: #289 Reviewed-By: Juan José Arboleda <[email protected]>
This commit introduces performance optimizations for tracing span attribute handling by adding two new fast API calls: 1. FastPushSpanDataString - Optimized version of the existing string push method 2. FastPushSpanDataString3 - New method to efficiently handle three string values in a single call Key changes include: - Avoided JavaScript string concatenation which produces non-flattened V8 strings that cannot trigger fast API paths - Implemented direct passing of individual string components to C++ where concatenation can happen after the fast API boundary - Refactored HTTP URL construction in both client and server to leverage this optimization - Improved span context handling by storing trace ID, span ID, and parent span ID as separate attributes rather than a single concatenated string These optimizations significantly improve performance in high load scenarios: - Up to 3.24% higher throughput in maximum load tests - Up to 57.75% lower latency at P99.9 under constant high rate (50k req/sec) - Substantial improvements across all high percentile latencies (P95-P99.999) - Most dramatic gains observed in tail latencies under sustained heavy load === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 40k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 2.58 | 2.34 | N/A | | | P50 Latency (ms) | 1.09 | 1.10 | +0.80% | *** | | P90 Latency (ms) | 2.22 | 2.20 | -0.63% | *** | | P99 Latency (ms) | 3.09 | 2.99 | -3.31% | *** | | Avg Req/Sec | 39,992.50 | 39,992.53 | +0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 50k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 9.92 | 5.20 | N/A | | | P50 Latency (ms) | 0.95 | 0.94 | -0.54% | | | P90 Latency (ms) | 2.54 | 2.35 | -7.47% | *** | | P99 Latency (ms) | 12.00 | 5.58 | -53.49% | *** | | Avg Req/Sec | 49,990.78 | 49,990.72 | -0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk (maximum throughput) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Threads: 2 +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 0.65 | 0.64 | N/A | | | P50 Latency (ms) | 0.17 | 0.16 | -3.48% | *** | | P90 Latency (ms) | 0.33 | 0.32 | -2.30% | *** | | P99 Latency (ms) | 0.36 | 0.35 | -3.33% | *** | | Avg Req/Sec | 52,398.20 | 54,094.55 | +3.24% | *** | +------------------+-------------+-------------+------------+-----+ PR-URL: #289 Reviewed-By: Juan José Arboleda <[email protected]>
This commit introduces performance optimizations for tracing span attribute handling by adding two new fast API calls: 1. FastPushSpanDataString - Optimized version of the existing string push method 2. FastPushSpanDataString3 - New method to efficiently handle three string values in a single call Key changes include: - Avoided JavaScript string concatenation which produces non-flattened V8 strings that cannot trigger fast API paths - Implemented direct passing of individual string components to C++ where concatenation can happen after the fast API boundary - Refactored HTTP URL construction in both client and server to leverage this optimization - Improved span context handling by storing trace ID, span ID, and parent span ID as separate attributes rather than a single concatenated string These optimizations significantly improve performance in high load scenarios: - Up to 3.24% higher throughput in maximum load tests - Up to 57.75% lower latency at P99.9 under constant high rate (50k req/sec) - Substantial improvements across all high percentile latencies (P95-P99.999) - Most dramatic gains observed in tail latencies under sustained heavy load === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 40k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 2.58 | 2.34 | N/A | | | P50 Latency (ms) | 1.09 | 1.10 | +0.80% | *** | | P90 Latency (ms) | 2.22 | 2.20 | -0.63% | *** | | P99 Latency (ms) | 3.09 | 2.99 | -3.31% | *** | | Avg Req/Sec | 39,992.50 | 39,992.53 | +0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk2 (constant rate) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Rate: 50k req/sec +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 9.92 | 5.20 | N/A | | | P50 Latency (ms) | 0.95 | 0.94 | -0.54% | | | P90 Latency (ms) | 2.54 | 2.35 | -7.47% | *** | | P99 Latency (ms) | 12.00 | 5.58 | -53.49% | *** | | Avg Req/Sec | 49,990.78 | 49,990.72 | -0.00% | | +------------------+-------------+-------------+------------+-----+ === Benchmark Summary === Benchmark Type: wrk (maximum throughput) Old Version: ./nsolid_baseline New Version: ./nsolid_fast_push_string Iterations: 50 Connections: 10 Duration: 60s Threads: 2 +------------------+-------------+-------------+------------+-----+ | Metric | Old Version | New Version | Difference | Sig | +------------------+-------------+-------------+------------+-----+ | Avg Latency (ms) | 0.65 | 0.64 | N/A | | | P50 Latency (ms) | 0.17 | 0.16 | -3.48% | *** | | P90 Latency (ms) | 0.33 | 0.32 | -2.30% | *** | | P99 Latency (ms) | 0.36 | 0.35 | -3.33% | *** | | Avg Req/Sec | 52,398.20 | 54,094.55 | +3.24% | *** | +------------------+-------------+-------------+------------+-----+ PR-URL: #289 Reviewed-By: Juan José Arboleda <[email protected]> PR-URL: #359 Reviewed-By: Rafael Gonzaga <[email protected]>
This commit introduces performance optimizations for tracing span attribute handling by adding two new fast API calls:
Key changes include:
These optimizations significantly improve performance in high load scenarios:
Summary by CodeRabbit
Refactor
New Features