Skip to content

Commit

Permalink
Addressing PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@jhoongo
jhoongo committed Mar 21, 2024
1 parent 48660d2 commit f69c8b2
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 90 deletions.
49 changes: 26 additions & 23 deletions Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/AdaptingCircularBufferCounter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,69 +15,68 @@ class AdaptingCircularBufferCounter: NSCopying {
return copy
}

private static let NULL_INDEX: Int = Int.min
public private(set) var endIndex = NULL_INDEX
public private(set) var startIndex = NULL_INDEX
private var baseIndex = NULL_INDEX
public private(set) var endIndex = Int.nullIndex
public private(set) var startIndex = Int.nullIndex
private var baseIndex = Int.nullIndex
private var backing: AdaptingIntegerArray
private let maxSize: Int

init(maxSize: Int) {
self.backing = AdaptingIntegerArray(size: maxSize)
backing = AdaptingIntegerArray(size: maxSize)
self.maxSize = maxSize
}

@discardableResult func increment(index: Int, delta: Int64) -> Bool{
if self.baseIndex == Int.min {
self.startIndex = index
self.endIndex = index
self.baseIndex = index
self.backing.increment(index: 0, count: delta)
if baseIndex == Int.min {
startIndex = index
endIndex = index
baseIndex = index
backing.increment(index: 0, count: delta)
return true
}

if index > self.endIndex {
if (index - self.startIndex + 1) > self.backing.length() {
if index > endIndex {
if (index - startIndex + 1) > backing.length() {
return false
}
self.endIndex = index
} else if index < self.startIndex {
if (self.endIndex - index + 1) > self.backing.length() {
endIndex = index
} else if index < startIndex {
if (endIndex - index + 1) > backing.length() {
return false

Check warning on line 45 in Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/AdaptingCircularBufferCounter.swift

View check run for this annotation

Codecov / codecov/patch

Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/AdaptingCircularBufferCounter.swift#L45 

Added line #L45 was not covered by tests
}
self.startIndex = index
}

let realIndex = toBufferIndex(index: index)
self.backing.increment(index: realIndex, count: delta)
backing.increment(index: realIndex, count: delta)
return true
}

func get(index: Int) -> Int64 {
if (index < self.startIndex || index > self.endIndex) {
if (index < startIndex || index > endIndex) {
return 0
} else {
return backing.get(index: toBufferIndex(index: index))
}
}

func isEmpty() -> Bool {
return baseIndex == Self.NULL_INDEX
return baseIndex == Int.nullIndex
}

func getMaxSize() -> Int {
return backing.length()
}

func clear() {
self.backing.clear()
self.baseIndex = Self.NULL_INDEX
self.startIndex = Self.NULL_INDEX
self.endIndex = Self.NULL_INDEX
backing.clear()
baseIndex = Int.nullIndex
startIndex = Int.nullIndex
endIndex = Int.nullIndex
}

private func toBufferIndex(index: Int) -> Int {
var result = index - self.baseIndex
var result = index - baseIndex
if (result >= backing.length()) {
result -= backing.length()

Check warning on line 81 in Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/AdaptingCircularBufferCounter.swift

View check run for this annotation

Codecov / codecov/patch

Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/AdaptingCircularBufferCounter.swift#L81 

Added line #L81 was not covered by tests
} else if (result < 0) {
Expand All @@ -86,3 +85,7 @@ class AdaptingCircularBufferCounter: NSCopying {
return result
}
}

extension Int {
static let nullIndex = Int.min
}
56 changes: 28 additions & 28 deletions Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/AdaptingIntegerArray.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ class AdaptingIntegerArray: NSCopying {

init(size: Int) {
self.size = size
self.cellSize = ArrayCellSize.byte
self.byteBacking = Array<Int8>(repeating: Int8(0), count: size)
cellSize = ArrayCellSize.byte
byteBacking = Array<Int8>(repeating: Int8(0), count: size)
}

func increment(index: Int, count: Int64) {

if self.cellSize == .byte, var byteBacking = self.byteBacking {
if cellSize == .byte, var byteBacking = self.byteBacking {
let result = Int64(byteBacking[index]) + count
if result > Int8.max {
resizeToShort()
Expand All @@ -55,7 +55,7 @@ class AdaptingIntegerArray: NSCopying {
byteBacking[index] = Int8(result)
self.byteBacking = byteBacking
}
} else if self.cellSize == .short, var shortBacking = self.shortBacking {
} else if cellSize == .short, var shortBacking = self.shortBacking {
let result = Int64(shortBacking[index]) + count
if result > Int16.max {
resizeToInt()
Expand All @@ -64,7 +64,7 @@ class AdaptingIntegerArray: NSCopying {
shortBacking[index] = Int16(result)
self.shortBacking = shortBacking
}
} else if self.cellSize == .int, var intBacking = self.intBacking {
} else if cellSize == .int, var intBacking = self.intBacking {
let result = Int64(intBacking[index]) + count
if result > Int32.max {
resizeToLong()
Expand All @@ -73,7 +73,7 @@ class AdaptingIntegerArray: NSCopying {
intBacking[index] = Int32(result)
self.intBacking = intBacking
}
} else if self.cellSize == .long, var longBacking = self.longBacking {
} else if cellSize == .long, var longBacking = self.longBacking {
let result = longBacking[index] + count
longBacking[index] = result
self.longBacking = longBacking
Expand All @@ -82,13 +82,13 @@ class AdaptingIntegerArray: NSCopying {

func get(index: Int) -> Int64 {

if self.cellSize == .byte, let byteBacking = self.byteBacking, index < byteBacking.count {
if cellSize == .byte, let byteBacking = self.byteBacking, index < byteBacking.count {
return Int64(byteBacking[index])
} else if self.cellSize == .short, let shortBacking = self.shortBacking, index < shortBacking.count {
} else if cellSize == .short, let shortBacking = self.shortBacking, index < shortBacking.count {
return Int64(shortBacking[index])
} else if self.cellSize == .int, let intBacking = self.intBacking, index < intBacking.count {
} else if cellSize == .int, let intBacking = self.intBacking, index < intBacking.count {
return Int64(intBacking[index])
} else if self.cellSize == .long, let longBacking = self.longBacking, index < longBacking.count {
} else if cellSize == .long, let longBacking = self.longBacking, index < longBacking.count {
return longBacking[index]
}

Expand All @@ -98,65 +98,65 @@ class AdaptingIntegerArray: NSCopying {
func length() -> Int {
var length = 0

if self.cellSize == .byte, let byteBacking = self.byteBacking {
if cellSize == .byte, let byteBacking = self.byteBacking {
length = byteBacking.count
} else if self.cellSize == .short, let shortBacking = self.shortBacking {
} else if cellSize == .short, let shortBacking = self.shortBacking {
length = shortBacking.count
} else if self.cellSize == .int, let intBacking = self.intBacking {
} else if cellSize == .int, let intBacking = self.intBacking {
length = intBacking.count
} else if self.cellSize == .long, let longBacking = self.longBacking {
} else if cellSize == .long, let longBacking = self.longBacking {
length = longBacking.count
}

return length
}

func clear() {
switch (self.cellSize) {
switch (cellSize) {
case .byte:
self.byteBacking = Array(repeating: Int8(0), count: self.byteBacking?.count ?? 0)
byteBacking = Array(repeating: Int8(0), count: byteBacking?.count ?? 0)
case .short:
self.shortBacking = Array(repeating: Int16(0), count: self.shortBacking?.count ?? 0)
shortBacking = Array(repeating: Int16(0), count: shortBacking?.count ?? 0)
case .int:
self.intBacking = Array(repeating: Int32(0), count: self.intBacking?.count ?? 0)
intBacking = Array(repeating: Int32(0), count: intBacking?.count ?? 0)
case .long:
self.longBacking = Array(repeating: Int64(0), count: self.longBacking?.count ?? 0)
longBacking = Array(repeating: Int64(0), count: longBacking?.count ?? 0)
}
}

private func resizeToShort() {
guard let byteBacking = self.byteBacking else { return }
guard let byteBacking = byteBacking else { return }
var tmpShortBacking: Array<Int16> = Array<Int16>(repeating: Int16(0), count: byteBacking.count)

for (index, value) in byteBacking.enumerated() {
tmpShortBacking[index] = Int16(value)
}
self.cellSize = ArrayCellSize.short
self.shortBacking = tmpShortBacking
cellSize = ArrayCellSize.short
shortBacking = tmpShortBacking
self.byteBacking = nil
}

private func resizeToInt() {
guard let shortBacking = self.shortBacking else { return }
guard let shortBacking = shortBacking else { return }
var tmpIntBacking: Array<Int32> = Array<Int32>(repeating: Int32(0), count: shortBacking.count)

for (index, value) in shortBacking.enumerated() {
tmpIntBacking[index] = Int32(value)
}
self.cellSize = ArrayCellSize.int
self.intBacking = tmpIntBacking
cellSize = ArrayCellSize.int
intBacking = tmpIntBacking
self.shortBacking = nil
}

private func resizeToLong() {
guard let intBacking = self.intBacking else { return }
guard let intBacking = intBacking else { return }
var tmpLongBacking: Array<Int64> = Array<Int64>(repeating: Int64(0), count: intBacking.count)

for (index, value) in intBacking.enumerated() {
tmpLongBacking[index] = Int64(value)
}
self.cellSize = ArrayCellSize.long
self.longBacking = tmpLongBacking
cellSize = ArrayCellSize.long
longBacking = tmpLongBacking
self.intBacking = nil
}
}
Expand Down
48 changes: 24 additions & 24 deletions ...enTelemetrySdk/Metrics/Stable/Aggregation/DoubleBase2ExponentialHistogramAggregator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class DoubleBase2ExponentialHistogramAggregator: StableAggregator {
}

Check warning on line 26 in Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/DoubleBase2ExponentialHistogramAggregator.swift

View check run for this annotation

Codecov / codecov/patch

Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/DoubleBase2ExponentialHistogramAggregator.swift#L24-L26 

Added lines #L24 - L26 were not covered by tests

public func createHandle() -> AggregatorHandle {
return Handle(maxBuckets: self.maxBuckets, maxScale: self.maxScale, exemplarReservoir: self.reservoirSupplier())
return Handle(maxBuckets: maxBuckets, maxScale: maxScale, exemplarReservoir: reservoirSupplier())
}

public func toMetricData(resource: Resource, scope: InstrumentationScopeInfo, descriptor: MetricDescriptor, points: [PointData], temporality: AggregationTemporality) -> StableMetricData {
Expand Down Expand Up @@ -73,28 +73,28 @@ public class DoubleBase2ExponentialHistogramAggregator: StableAggregator {
}

let pointData = ExponentialHistogramPointData(
scale: self.scale,
sum: self.sum,
zeroCount: Int64(self.zeroCount),
hasMin: self.count > 0,
hasMax: self.count > 0,
min: self.min,
max: self.max,
positiveBuckets: resolveBuckets(buckets: self.positiveBuckets, scale: self.scale, reset: reset),
negativeBuckets: resolveBuckets(buckets: self.negativeBuckets, scale: self.scale, reset: reset),
scale: scale,
sum: sum,
zeroCount: Int64(zeroCount),
hasMin: count > 0,
hasMax: count > 0,
min: min,
max: max,
positiveBuckets: resolveBuckets(buckets: positiveBuckets, scale: scale, reset: reset),
negativeBuckets: resolveBuckets(buckets: negativeBuckets, scale: scale, reset: reset),
startEpochNanos: startEpochNano,
epochNanos: endEpochNano,
attributes: attributes,
exemplars: exemplars
)

if reset {
self.sum = 0
self.zeroCount = 0
self.min = Double.greatestFiniteMagnitude
self.max = -1
self.count = 0
self.scale = self.maxScale
sum = 0
zeroCount = 0
min = Double.greatestFiniteMagnitude
max = -1
count = 0
scale = maxScale
}

return pointData
Expand Down Expand Up @@ -124,15 +124,15 @@ public class DoubleBase2ExponentialHistogramAggregator: StableAggregator {
if let positiveBuckets = self.positiveBuckets {
buckets = positiveBuckets
} else {
buckets = DoubleBase2ExponentialHistogramBuckets(scale: self.scale, maxBuckets: self.maxBuckets)
self.positiveBuckets = buckets
buckets = DoubleBase2ExponentialHistogramBuckets(scale: scale, maxBuckets: maxBuckets)
positiveBuckets = buckets
}
} else {
if let negativeBuckets = self.negativeBuckets {
if let negativeBuckets = negativeBuckets {
buckets = negativeBuckets
} else {
buckets = DoubleBase2ExponentialHistogramBuckets(scale: self.scale, maxBuckets: self.maxBuckets)
self.negativeBuckets = buckets
buckets = DoubleBase2ExponentialHistogramBuckets(scale: scale, maxBuckets: maxBuckets)
negativeBuckets = buckets
}
}

Expand All @@ -150,20 +150,20 @@ public class DoubleBase2ExponentialHistogramAggregator: StableAggregator {
let copy = buckets.copy() as! DoubleBase2ExponentialHistogramBuckets

if reset {
buckets.clear(scale: self.maxScale)
buckets.clear(scale: maxScale)
}

return copy
}

func downScale(by: Int) {
if let positiveBuckets = self.positiveBuckets {
if let positiveBuckets = positiveBuckets {
positiveBuckets.downscale(by: by)
scale = positiveBuckets.scale

}

if let negativeBuckets = self.negativeBuckets {
if let negativeBuckets = negativeBuckets {
negativeBuckets.downscale(by: by)
scale = negativeBuckets.scale

Check warning on line 168 in Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/DoubleBase2ExponentialHistogramAggregator.swift

View check run for this annotation

Codecov / codecov/patch

Sources/OpenTelemetrySdk/Metrics/Stable/Aggregation/DoubleBase2ExponentialHistogramAggregator.swift#L167-L168 

Added lines #L167 - L168 were not covered by tests
}
Expand Down
27 changes: 15 additions & 12 deletions Sources/OpenTelemetrySdk/Metrics/Stable/Data/Internal/Base2ExponentialHistogramIndexer.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,13 @@ import OpenTelemetryApi
public class Base2ExponentialHistogramIndexer {
private static var cache = [Int: Base2ExponentialHistogramIndexer]()
private static var cacheLock = Lock()
private static let LOG_BASE2_E = 1.0 / log(2)
private static let EXPONENT_BIT_MASK : Int = 0x7FF0_0000_0000_0000
private static let SIGNIFICAND_BIT_MASK : Int = 0xF_FFFF_FFFF_FFFF
private static let EXPONENT_BIAS : Int = 1023
private static let SIGNIFICAND_WIDTH : Int = 52
private static let EXPONENT_WIDTH : Int = 11

private let scale : Int
private let scaleFactor : Double

init(scale: Int) {
self.scale = scale
self.scaleFactor = Self.computeScaleFactor(scale: scale)
scaleFactor = Self.computeScaleFactor(scale: scale)
}

func get(_ scale: Int) -> Base2ExponentialHistogramIndexer {
Expand Down Expand Up @@ -55,20 +49,29 @@ public class Base2ExponentialHistogramIndexer {

func mapToIndexScaleZero(_ value : Double) -> Int {
let raw = value.bitPattern
var rawExponent = (Int(raw) & Self.EXPONENT_BIT_MASK) >> Self.SIGNIFICAND_WIDTH // does `value.exponentBitPattern` work here?
let rawSignificand = Int(raw) & Self.SIGNIFICAND_BIT_MASK // does `value.significandBitPattern` work here?
var rawExponent = Int((Int64(raw) & Int64(0x7FF0_0000_0000_0000)) >> Int.significandWidth)
let rawSignificand = Int(Int64(raw) & Int64(0xF_FFFF_FFFF_FFFF))
if rawExponent == 0 {
rawExponent -= (rawSignificand - 1).leadingZeroBitCount - Self.EXPONENT_WIDTH - 1
rawExponent -= (rawSignificand - 1).leadingZeroBitCount - Int.exponentWidth - 1

Check warning on line 55 in Sources/OpenTelemetrySdk/Metrics/Stable/Data/Internal/Base2ExponentialHistogramIndexer.swift

View check run for this annotation

Codecov / codecov/patch

Sources/OpenTelemetrySdk/Metrics/Stable/Data/Internal/Base2ExponentialHistogramIndexer.swift#L55 

Added line #L55 was not covered by tests
}
let ieeeExponent = rawExponent - Self.EXPONENT_BIAS
let ieeeExponent = rawExponent - Int.exponentBias
if rawSignificand == 0 {
return ieeeExponent - 1
}
return ieeeExponent
}

static func computeScaleFactor(scale: Int) -> Double {
Self.LOG_BASE2_E * pow(2.0, Double(scale))
Double.logBase2E * pow(2.0, Double(scale))
}
}

extension Int {
static let exponentBias = 1023
static let significandWidth = 52
static let exponentWidth = 11
}

extension Double {
static let logBase2E = 1.0 / log(2)
}
Loading

0 comments on commit f69c8b2

Please sign in to comment.