Skip to content

LG-8060 | Timing WIP#7598

Closed
n1zyy wants to merge 3 commits intomainfrom
mattw/lg-8060-wip
Closed

LG-8060 | Timing WIP#7598
n1zyy wants to merge 3 commits intomainfrom
mattw/lg-8060-wip

Conversation

@n1zyy
Copy link
Contributor

@n1zyy n1zyy commented Jan 6, 2023

I've focused on the slowest parts, and been unable to speed them up. Pushing this up just for discussion right now.

I've focused on the slowest parts, and been unable to speed them up.
Pushing this up just for discussion right now.
Copy link
Contributor Author

@n1zyy n1zyy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that all of this is experimentation. Lots and lots of it has room for improvement, but I'm just trying to show some of what I've poked at. Really my conclusion here is "I'm not sure this is a worthwhile endeavor," so please don't waste a lot of cycles trying to improve my timekeeping code.

puts "#{elapsed} sec. from '#{@previous_timer}' to '#{name}'"
end
@previous_timer = name
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore this mediocre method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you seen the JobHelpers::Timer class we have? example usage in

result = timer.time('threatmetrix') do
lexisnexis_ddp_proofer.proof(ddp_pii)
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did, but not until I'd already written this. 😿

f.write(data)
`gzip #{f.path}`
timestamp(:gzip_shell)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is dumb and not using dir at all, and I wouldn't interpolate unescaped arguments inside backticks if this code were going anywhere. But, the outcome of this is that shelling out to gzip here is not any faster than when we call Zlib.gzip above. It takes the same amount of time.


key = cipher.random_key
iv = cipher.random_iv
timestamp(:get_and_iv_assignment)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
timestamp(:get_and_iv_assignment)
timestamp(:key_and_iv_assignment)

But this is just insignificant instrumentation that shouldn't be merged.

file.close
timestamp(:large_file_write)
`xxd -p -c 0 base16_me.txt > base16_done.txt`
timestamp(:xxd)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this is sloppy experimental code, but the outcome is that the call to xxd is taking about 10x longer than the Base16.encode16 call. I'm curious to dig into why, but on the surface, this does not look like a promising speed boost.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What sizes of data did you use for these tests? Previously it took until like 100k batches for the shell to win

#7274 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did 260k events. (10k, then I added 250k because it was too small.)

)
end
puts 'Done!'
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is shockingly slow; about 3 minutes and CPU-bound. I should move the event generation outside the loop, including the .to_jwe call, so the inside of the loop is just writing static strings to Redis.

Initially this was 10k instead of 250k and the couple seconds it took was immaterial. I wanted the data to be quasi-realistic.

@n1zyy
Copy link
Contributor Author

n1zyy commented Jan 26, 2023

Closing out as most of this failed to give any performance increase. Replaced by #7706

@n1zyy n1zyy closed this Jan 26, 2023
@n1zyy n1zyy deleted the mattw/lg-8060-wip branch October 11, 2023 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants