Micro optimization: build Hash w/ {}

[jbourassa]

For cases where all keys are present, create the Hash with all its keys with {...} instead of building it incrementally with multiple Hash[] calls.

It really is a micro-optimization, but execute_field is on the hot path.

Bench:

require "benchmark/ips"

Benchmark.ips do |x|
  x.report("[]=") do
    hash = {}
    hash["a"] = 1
    hash["b"] = 2
    hash["c"] = 3
  end

  x.report("{}") do
    { "a" => 1, "b" => 2, "c" => 3 }
  end

  x.compare

Result:

Warming up --------------------------------------
                  []=  577.660k i/100ms
                  {}     1.438M i/100ms
Calculating -------------------------------------
                  []=     5.809M (± 2.9%) i/s -     29.461M in   5.076542s
                  {}     15.373M (± 0.3%) i/s -     77.651M in   5.051278s

Comparison:
                  {}=: 15372655.4 i/s
                  []:   5808733.8 i/s - 2.65x  slower

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: jbourassa / name: Jimmy Bourassa (c9b4187)

[jbourassa]

Unrelated to this exact diff but is there a reason we can't cache this, like it's done in other fields? I see it depends on field.trace, field.type, @trace_scalars, all of which look stable.

[arielvalentin]

That would be better answered by @swalkinshaw or @rmosolgo

[swalkinshaw]

The actual key lookup is cached within the method, but yeah trace_field? is called each time and I think it could be cached 👍

[arielvalentin]

While I have your eyes on this @swalkinshaw... would you mind providing a review?

[swalkinshaw]

This was carried over from the other file I ported where the individual assigment was necessary:

opentelemetry-ruby-contrib/instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_tracer.rb

Lines 88 to 107 in d77b700

	def attributes_for(key, data)
	attributes = {}
	case key
	when 'execute_field', 'execute_field_lazy'
	attributes['graphql.field.parent'] = data[:owner]&.graphql_name # owner is the concrete type, not interface
	attributes['graphql.field.name'] = data[:field]&.graphql_name
	attributes['graphql.lazy'] = key == 'execute_field_lazy'
	when 'authorized', 'authorized_lazy'
	attributes['graphql.type.name'] = data[:type]&.graphql_name
	attributes['graphql.lazy'] = key == 'authorized_lazy'
	when 'resolve_type', 'resolve_type_lazy'
	attributes['graphql.type.name'] = data[:type]&.graphql_name
	attributes['graphql.lazy'] = key == 'resolve_type_lazy'
	when 'execute_query'
	attributes['graphql.operation.name'] = data[:query].selected_operation_name if data[:query].selected_operation_name
	attributes['graphql.operation.type'] = data[:query].selected_operation.operation_type
	attributes['graphql.document'] = data[:query].query_string
	end
	attributes
	end

So this makes sense anyway even if it wasn't faster!

[arielvalentin]

Something I did not notice before is that this line and owner may be nil.

That will result in a noisy error handling when trying to add the attributes to the span:

https://github.com/open-telemetry/opentelemetry-ruby/blob/37e225ac63fef4617fc78bf1f638417eeb6a6e91/sdk/lib/opentelemetry/sdk/internal.rb#L57

A compact!, or equivalent, would be appreciated before sending this along to in_span