These are chat archives for exceptionless/Discuss

4th
Feb 2016
@niemyjski we should be looking at that in the client and server side to better uniquely identify requests coming from behind proxies.
Blake Niemyjski
@niemyjski
Feb 04 2016 14:22
yeah
would be good
Sander Rijken
@srijken
Feb 04 2016 14:27
sweet
Blake Niemyjski
@niemyjski
Feb 04 2016 14:27
yeah
Blake Niemyjski
@niemyjski
Feb 04 2016 14:39
Hi,
It can be used for custom Exception Handler client side on my asp.net core1 project.
I'm not good on angular2 I start to learn it and I need to move forward in my project.
Thanks.
we need to spend time on that
exceptionless/Exceptionless.Net#28
Blake Niemyjski
@niemyjski
Feb 04 2016 14:52
@rbaz you can use it client side
But we don't yet have a client targeting coreclr
Blake Niemyjski
@niemyjski
Feb 04 2016 17:10
@ejsmith I’m going to take the Exceptiones Data property and loop through it and add the items to the errors data property
so it’s converted to the root
@srijken this should help a bit with deduplication :) exceptionless/Exceptionless.Net@564d6eb
Blake Niemyjski
@niemyjski
Feb 04 2016 17:26
var exclusions = _exceptionExclusions.Union(client.Configuration.DataExclusions).ToList();
try {
if (exception.Data != null) {
foreach (object k in exception.Data.Keys) {
string key = k as string;
if (String.IsNullOrEmpty(key) || key.AnyWildcardMatches(exclusions, true))
continue;
                    error.Data[key] = exception.Data[k];
                }
            }
        } catch (Exception ex) {
            log.Error(typeof(ExceptionlessClient), ex, "Error populating TargetMethod: " + ex.Message);
        }
it will move it all up a level
instead of error.data.data
Sander Rijken
@srijken
Feb 04 2016 18:01
@niemyjski I’m missing context here.. help with deduplication how?
Blake Niemyjski
@niemyjski
Feb 04 2016 18:19
were going to start marking exceptions as processed via the data property
so if it gets rethrown
it will already be marked
so those cases where the stack traces are different
are now covered :)
Blake Niemyjski
@niemyjski
Feb 04 2016 19:06
W000T found a few test cases we didn’t support. should be pretty rock solid now :D
exceptionless/Exceptionless.Net@ec14cb7
Sander Rijken
@srijken
Feb 04 2016 19:48
cool
Eric J. Smith
@ejsmith
Feb 04 2016 19:50
nice
good job
Sander Rijken
@srijken
Feb 04 2016 19:53
any suggestions how to test that the duplicate checker plugin correctly sets the value?
I have to make the timer event configurable somehow, so it fires very quickly
Blake Niemyjski
@niemyjski
Feb 04 2016 19:54
I’d make the interval be a constructor value
that defaults to our interval
then we can just test it
make sense?
Sander Rijken
@srijken
Feb 04 2016 19:55
yup
Sander Rijken
@srijken
Feb 04 2016 20:02
hmm the timer event isn’t being hit
Blake Niemyjski
@niemyjski
Feb 04 2016 20:03
it won’t be until an event goes through it
Sander Rijken
@srijken
Feb 04 2016 20:05
I was passing -1 in the dueTime parameter
I misunderstood the docs
Blake Niemyjski
@niemyjski
Feb 04 2016 20:05
ah
yeah
that’s tricky
brb
Just pushed a new tracekit release that had a few good bug fixes that users had contributed over the last few months
Sander Rijken
@srijken
Feb 04 2016 20:07
you also maintain tracekit?
Blake Niemyjski
@niemyjski
Feb 04 2016 20:08
yep
Blake Niemyjski
@niemyjski
Feb 04 2016 21:03
@srijken what can I help you with deducing?
Sander Rijken
@srijken
Feb 04 2016 21:04
hm?
it works now
Blake Niemyjski
@niemyjski
Feb 04 2016 21:04
@ejsmith @srijken how do we feel about this exceptionless/Exceptionless.Net#25
is everything pushed to that branch?
Sander Rijken
@srijken
Feb 04 2016 21:05
yes
Blake Niemyjski
@niemyjski
Feb 04 2016 21:05
last commit from you is 20 days ago
Sander Rijken
@srijken
Feb 04 2016 21:05
I just pushed something
like last 30 minutes, or do you mean something else?
and about that issue.. I find it highly unlikely that de dupe checker found any duplicates
Blake Niemyjski
@niemyjski
Feb 04 2016 21:06
Screen Shot 2016-02-04 at 3.06.02 PM.png
Sander Rijken
@srijken
Feb 04 2016 21:07
did you sync your local repository?
Blake Niemyjski
@niemyjski
Feb 04 2016 21:07
yes
did you do all of this in your own fork or branch on main repo?
it’s there
and ok ok, it was an hour ago
I really think your local repository is behind?
Blake Niemyjski
@niemyjski
Feb 04 2016 21:10
I synced
Sander Rijken
@srijken
Feb 04 2016 21:11
hmm
what else can it be
I mean, you see them at github right?
Blake Niemyjski
@niemyjski
Feb 04 2016 21:13
yup
Sander Rijken
@srijken
Feb 04 2016 21:13
I don’t get it
Blake Niemyjski
@niemyjski
Feb 04 2016 21:13
changes are locally in mine just not in the lisat
Sander Rijken
@srijken
Feb 04 2016 21:14
weird
I’m not a git specialist :)
what does commandline git say about it?
Blake Niemyjski
@niemyjski
Feb 04 2016 21:19
this duplicate checker plugin is being applied to every event type
do we want that to happen right now
seems like we’d only want it applied to errors...
Sander Rijken
@srijken
Feb 04 2016 21:22
why
Blake Niemyjski
@niemyjski
Feb 04 2016 21:23
because if I’m sending in log messages
I might be checking for 10
if I’m debugging something
Sander Rijken
@srijken
Feb 04 2016 21:24
hm
I think we discussed this, or with @ejsmith, I’ll try to dig back
Blake Niemyjski
@niemyjski
Feb 04 2016 21:26
ok
I just pushed a small change in formatting and changed to use timespan as interval. I think we make a change to timespan as it’s making the code more readable across our projects. You don’t really know what the int represents by looking at it
So after reviewing the code I had the question…. We are storing 10 hashes in the queue.. feels like this could be more.. (we call it _recentlyProcessedErrors… shouldn’t it be recentlyProcessedEvents?? ) but let me start by this..
lets say we submit 10 events 1 of them are duplicates. So 9 went through and 1 is in the queuue for holding.. Then we submit 2 new events and lets assume that 1 of those hashes getting popped off the queue is the one that’s in the queue for holding… Now a new duplicate that matches that queue item for holding comes in, but not in recently processed events...
Sander Rijken
@srijken
Feb 04 2016 21:31
it used to only handle errors
Blake Niemyjski
@niemyjski
Feb 04 2016 21:31
hard to follow but do you see what I’m saying.. It’s almost like any queued hashcodes never get popped off until the count is populated and they should only be popped off if the enqueue time > 10 seconds since last active
trying to keep this simple though
Sander Rijken
@srijken
Feb 04 2016 21:33
if it’s still in the queue, it’ll increase the count
if the queue has been processed it’ll add it again
Blake Niemyjski
@niemyjski
Feb 04 2016 21:33
just seems like theres a gap where something will be in the queued to send but also out of the recently processed and then put back as a duplicate into the queue again
ok
Sander Rijken
@srijken
Feb 04 2016 21:33
there’s 2 queues right
one for detection, one for sending
Blake Niemyjski
@niemyjski
Feb 04 2016 21:34
yep
Sander Rijken
@srijken
Feb 04 2016 21:34
one for sending is only popped in the timer
so the dupe is either popped or not popped.. it doesn’t matter if it’s in the detection queue or not
so what can happen is:
  • single event comes through (no dupe yet)
  • several other dupes, count increases by every dupe
  • several non-dupes (these clear the detection queue)
  • another dupe, but now we don’t know it’s a dupe yet, so that gets through right away, other bunch is still holding
  • next dupes are added to the one still queued
  • timer hits, and the queued one gets sent off
I think that’s what you were trying to say ?
Blake Niemyjski
@niemyjski
Feb 04 2016 21:37
I need to copy that to the issue so it’s there and not in gitter :)
Sander Rijken
@srijken
Feb 04 2016 21:37
yeah :D
you can also make a second one where the first dupe bunch is already sent when the next dupe comes in :)
Blake Niemyjski
@niemyjski
Feb 04 2016 21:39
ok
I think I see where there is a bug
Sander Rijken
@srijken
Feb 04 2016 21:39
that’s good :D
Blake Niemyjski
@niemyjski
Feb 04 2016 21:39
// Keep count of number of times the duplication occurs
var recent = _recentDuplicates.FirstOrDefault(s => s.HashCode == hashCode);
if (recent != null) {
recent.IncrementCount();
context.Log.FormattedInfo(typeof(ExceptionlessClient), "Ignoring duplicate error event: hash={0}", hashCode);
                return;
            }
that code
needs to happen every time
because right now it’s only happening if the event is in the recently processed list (small and short lived)
vs incrementing it in a long lived recent duplicates
Sander Rijken
@srijken
Feb 04 2016 21:40
ok let me think
if it’s not in the short-lived list, it’s not detected as a dupe, and shouldn’t be incremented?
Blake Niemyjski
@niemyjski
Feb 04 2016 21:41
it should be
and it should be canceled
as it’s a duplicate that’s queued for sending
we should increment and cancel it
Sander Rijken
@srijken
Feb 04 2016 21:41
yeah, right now it doesn’t increment and doesn’t cancel
that’s kind of what I just described too
Blake Niemyjski
@niemyjski
Feb 04 2016 21:44
yeah
Sander Rijken
@srijken
Feb 04 2016 21:44
not sure if it’s a big problem or not.. it depends on how agressive we want to be on this
no information is being lost
Blake Niemyjski
@niemyjski
Feb 04 2016 21:47
yeah
so our long lived duplicates run every 30 seconds
but the repeat window is 2 seconds
dam
seems like a crazy time difference
Sander Rijken
@srijken
Feb 04 2016 21:48
it’s just arbitrary numbers
Blake Niemyjski
@niemyjski
Feb 04 2016 21:48
so I get a duplicate and then I don’t see it for a half minute but if it goes out of the short queue I see it right away
Sander Rijken
@srijken
Feb 04 2016 21:48
so right now if the dupe keeps happening all the time, you get an aggregate event every 30 seconds
Blake Niemyjski
@niemyjski
Feb 04 2016 21:49
not exactly
Sander Rijken
@srijken
Feb 04 2016 21:49
and if it stops happening for 2 seconds and 10 others come in, it’ll no longer be seen as a dupe
Blake Niemyjski
@niemyjski
Feb 04 2016 21:50
if I send 20 events if a few of them our duplicates in the first 5 of that batch the 1st one will be sent, 2nd will be queued and third will be canceled (it will be sent 30 seconds later). But the one that happens say #19 will be sent right away then 25+ seconds later 2nd will come through with a value of 2
are* duplicates
Sander Rijken
@srijken
Feb 04 2016 21:51
yeah
maybe making this change to untangle the 2 queues will make the code even simpler, which is always a good thing
you start by checking the long-lived one, if you find it there, handle it there
Blake Niemyjski
@niemyjski
Feb 04 2016 21:54
yeah
Sander Rijken
@srijken
Feb 04 2016 21:54
don’t find it -> use the short-live queue for detection
Blake Niemyjski
@niemyjski
Feb 04 2016 21:54
I almost have that done
want to do a teamveiwer and work on this together?
Sander Rijken
@srijken
Feb 04 2016 21:55
I’m also working on something else, but just watching can’t hurt :)
it’s a slow process I’m working on anyway
Blake Niemyjski
@niemyjski
Feb 04 2016 21:58
397 512 836
Sander Rijken
@srijken
Feb 04 2016 22:00
looks like it times out
hm
seems there’s a problem
can you restart teamviewer?
Blake Niemyjski
@niemyjski
Feb 04 2016 22:04
slow
Sander Rijken
@srijken
Feb 04 2016 22:06
right
it’s so damn small I can’t see anything lol
oh theater mode helps a little bit
Blake Niemyjski
@niemyjski
Feb 04 2016 22:07
that better
Sander Rijken
@srijken
Feb 04 2016 22:07
that helps too
Blake Niemyjski
@niemyjski
Feb 04 2016 22:07
can you hear my music?
Sander Rijken
@srijken
Feb 04 2016 22:07
yeah
Blake Niemyjski
@niemyjski
Feb 04 2016 22:07
letm e know if you want me to kill it
Sander Rijken
@srijken
Feb 04 2016 22:08
it’s ok, I can control the volume :)
Blake Niemyjski
@niemyjski
Feb 04 2016 22:08
I paused it and am talking
Sander Rijken
@srijken
Feb 04 2016 22:08
can’t talk back but I hear you
uhm
Eric J. Smith
@ejsmith
Feb 04 2016 22:22
Yes, I want it applied to all events. I want dupes to be rolled up and value incremented.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:27
@niemyjski
@ejsmith
for deduping
I tihnk we should limit it to error type events for now
until we know the impact of gethashocode
we don’t know how much slower it will be
secondly were queueing thigns for 5 seconds
should that window be smaller for dups
Eric J. Smith
@ejsmith
Feb 04 2016 22:27
why would we limit it?
if it’s a dupe it’s a dupe
Blake Niemyjski
@niemyjski
Feb 04 2016 22:27
because gethashcode could be expensive
Eric J. Smith
@ejsmith
Feb 04 2016 22:27
increment the value
Blake Niemyjski
@niemyjski
Feb 04 2016 22:27
we’ve yet to know
also we aren’t rolling up values
Eric J. Smith
@ejsmith
Feb 04 2016 22:28
it’s not going to be expensive especially not on smaller events.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:28
that’s an assumption
were doing this every 5 seconds
rolling up every duplicate for 5 seconds which I feel is bad
Eric J. Smith
@ejsmith
Feb 04 2016 22:28
no, I want to be more aggressive with rolling dupes up.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:28
for example, what if you wanted to know a counter on a per second basis
Eric J. Smith
@ejsmith
Feb 04 2016 22:28
it sends the 1st one right through.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:28
they are now in 5 second buckets
yeah
Eric J. Smith
@ejsmith
Feb 04 2016 22:29
I would be completely fine with being much more aggressive like 60 seconds.
you get the 1st one in real time… so it feels very real time and then you are getting rollups after that.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:29
this seems like very opt in
Eric J. Smith
@ejsmith
Feb 04 2016 22:29
why?
Blake Niemyjski
@niemyjski
Feb 04 2016 22:30
because I might want every event especially for debugging
Eric J. Smith
@ejsmith
Feb 04 2016 22:30
dude… you are getting every event. this is rolling up exact duplicates and still only holding on to them for maybe 60 seconds.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:30
yeah
right now we bail if event.value is set
Eric J. Smith
@ejsmith
Feb 04 2016 22:31
good
because we need to use that
if it’s set then we can’t roll up
Blake Niemyjski
@niemyjski
Feb 04 2016 22:31
ok
Eric J. Smith
@ejsmith
Feb 04 2016 22:31
because we are using that to roll up
Blake Niemyjski
@niemyjski
Feb 04 2016 22:31
so want me to store these for 60 seconds
or 30
Eric J. Smith
@ejsmith
Feb 04 2016 22:31
yes and we should only be storing the hash of the 1st timers...
Blake Niemyjski
@niemyjski
Feb 04 2016 22:32
hash and time
or hash only
Eric J. Smith
@ejsmith
Feb 04 2016 22:32
the 2nd one that comes through that gets a hash hit is when we would store the entire event to be sent later.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:32
join my livecoding channel
Eric J. Smith
@ejsmith
Feb 04 2016 22:32
can't
Eric J. Smith
@ejsmith
Feb 04 2016 22:32
I got fires over here and it’s my daughters birthday tonigh.
Sander Rijken
@srijken
Feb 04 2016 22:33
say you’re gonna store for 60 seconds. does that imply that the detection timespan is also 60 seconds?
Eric J. Smith
@ejsmith
Feb 04 2016 22:33
yes, I want to rollup as many as possible.
you really probably arem
aren’t going to get a ton of hits because username will most likely be different.
wish there was something that we could do about that.
Sander Rijken
@srijken
Feb 04 2016 22:34
you want those seperated
Eric J. Smith
@ejsmith
Feb 04 2016 22:34
yeah
have to
but I wish we could roll them up somehow.
but that is a different battle. :-)
the goal here is to figure out ways to send the least amount of events while providing the most values to users.
so the more we can roll up the better.
Sander Rijken
@srijken
Feb 04 2016 22:37
I really do hope that all the GetHashCode() calls calling each other don’t add up to too much time
Blake Niemyjski
@niemyjski
Feb 04 2016 22:37
yeah
that’s my worry
Sander Rijken
@srijken
Feb 04 2016 22:37
but this is a queue on a bg thread right?
Blake Niemyjski
@niemyjski
Feb 04 2016 22:37
which is why I’d love to limit this to errors right now
Sander Rijken
@srijken
Feb 04 2016 22:37
you don’t want that filling up though
Eric J. Smith
@ejsmith
Feb 04 2016 22:37
yes, it’s happening on a bg thread.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:38
errors happen at random..
developers control there logs
Eric J. Smith
@ejsmith
Feb 04 2016 22:38
and the time that it will take to do hashcodes will be very small compared to any code doing real work.
Sander Rijken
@srijken
Feb 04 2016 22:38
I’ve hit some errors happening so fast that I’m sure there would’ve been a delay.. or at least a good benchmark test for this :D
it’s far faster than sending all the http requests one by one for sure
Eric J. Smith
@ejsmith
Feb 04 2016 22:39
exactly
Blake Niemyjski
@niemyjski
Feb 04 2016 22:39
so question
eh never mind
dumb question
Sander Rijken
@srijken
Feb 04 2016 22:39
lol
Blake Niemyjski
@niemyjski
Feb 04 2016 22:40
was going to ask why we need a time on the processed queue
but its because if they aren’t very active it should be ignored
:)
Sander Rijken
@srijken
Feb 04 2016 22:40
and then you figured it out :)
Blake Niemyjski
@niemyjski
Feb 04 2016 22:40
yeah
Eric J. Smith
@ejsmith
Feb 04 2016 22:40
we should use the timer pattern that we are using in the in memory queues and other things from foundatio
where the timer only fires exactly when it needs to.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:40
eh
once every 60 seconds
not really worried about that...
Sander Rijken
@srijken
Feb 04 2016 22:41
and the only thing it does is try to dequeue an event
Blake Niemyjski
@niemyjski
Feb 04 2016 22:41
ooh crap just thought of something
Sander Rijken
@srijken
Feb 04 2016 22:41
it’s not doing a ton of work
Blake Niemyjski
@niemyjski
Feb 04 2016 22:42
if we are waiting 60 seconds
then shutting down randomly
become a bigger issue
as you just lost a minute of data
Sander Rijken
@srijken
Feb 04 2016 22:42
other thing.. I think .Send() needs to be wrapped in a try{} catch{}?
Eric J. Smith
@ejsmith
Feb 04 2016 22:42
it should send before dispose
which will put them in the queue
if they have persistent queues… they will be saved.
Sander Rijken
@srijken
Feb 04 2016 22:43
you don’t want exceptions to break out of the timer event.. it’ll crash the app hard
Eric J. Smith
@ejsmith
Feb 04 2016 22:43
yeah
we have to be super careful to not crash people’s apps. :-)
Blake Niemyjski
@niemyjski
Feb 04 2016 22:43
no
they are in memory queues inside of the plugin
and will sit there for up to 1 minute
Eric J. Smith
@ejsmith
Feb 04 2016 22:44
yes, I know… but dispose gets called.
Sander Rijken
@srijken
Feb 04 2016 22:44
yeah but if you send it from the dispose, you can send it earlier
Blake Niemyjski
@niemyjski
Feb 04 2016 22:44
hmm
Eric J. Smith
@ejsmith
Feb 04 2016 22:44
it needs to submit any pending in the dispose
Blake Niemyjski
@niemyjski
Feb 04 2016 22:44
yeah
Sander Rijken
@srijken
Feb 04 2016 22:44
except when dispose doesn’t get called..
Blake Niemyjski
@niemyjski
Feb 04 2016 22:44
good call
Sander Rijken
@srijken
Feb 04 2016 22:44
but then it just sucks anyway
Eric J. Smith
@ejsmith
Feb 04 2016 22:44
in that case we are screwed anyway
Blake Niemyjski
@niemyjski
Feb 04 2016 22:44
but will dispose actually get called on shutdown
might be wayyyyy too late
Eric J. Smith
@ejsmith
Feb 04 2016 22:45
I think it will be ok… but try it.
and it would be nice if we did a quick benchmark on calling gethashcode on complex events.
Sander Rijken
@srijken
Feb 04 2016 22:46
we also need to add that to the testcase to make sure it even works :D
Eric J. Smith
@ejsmith
Feb 04 2016 22:46
because there are discussions about making architectural decisions based on this being expensive and I don’t believe in making architectural decisions based on milliseconds. :-)
Sander Rijken
@srijken
Feb 04 2016 22:46
and without measuring in the first place
Blake Niemyjski
@niemyjski
Feb 04 2016 22:46
yeah
Eric J. Smith
@ejsmith
Feb 04 2016 22:47
exactly
Blake Niemyjski
@niemyjski
Feb 04 2016 22:47
I’ll add a test to the master branch
and then pull it into this one
I’ll make that a toodo
Sander Rijken
@srijken
Feb 04 2016 22:47
ex collegue used to do that.. optimize for optimal reuse (and then it doesn’t get reused), and for optimal speed, when speed isn’t the issue there
Blake Niemyjski
@niemyjski
Feb 04 2016 22:47
yeah
I know
I’ll write a test and add some good test cases
Sander Rijken
@srijken
Feb 04 2016 22:47
especially the first thing leads to code more complex than necessary
livecodign has delay :) I see the message come in, and then I see you typing it
Eric J. Smith
@ejsmith
Feb 04 2016 22:48
drives me insane when people make architectural design decisions based on microbenchmarks.
build for simplicity and maintenance… profile the entire thing and fix the real hotspots.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:50
you really think I need to do smart timers
lots of extra code for just this that runs every 60 seconds
Eric J. Smith
@ejsmith
Feb 04 2016 22:50
probably not
Blake Niemyjski
@niemyjski
Feb 04 2016 22:50
kk
sounds good
Eric J. Smith
@ejsmith
Feb 04 2016 22:50
its kind of nice to only fire when you need to, but probably not worth it.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:50
I’ll commit these changes and we’ll have to test this
yeah
I don’t think it’s worth it
Eric J. Smith
@ejsmith
Feb 04 2016 22:54
sounds good.
Blake Niemyjski
@niemyjski
Feb 04 2016 22:55
we gotta make sure we don’t use any c# 6 features in this client or it breaks the build
Sander Rijken
@srijken
Feb 04 2016 22:55
ah
oh right that thing
Blake Niemyjski
@niemyjski
Feb 04 2016 22:58
Any chance you could update all occurrences of ?.
Sander Rijken
@srijken
Feb 04 2016 22:58
that sucks.. wish I knew about this
Blake Niemyjski
@niemyjski
Feb 04 2016 22:58
yeah :(
Sander Rijken
@srijken
Feb 04 2016 22:59
I didn’t see the build turn red either when I committed stuff
brb
Blake Niemyjski
@niemyjski
Feb 04 2016 23:11
k
Sander Rijken
@srijken
Feb 04 2016 23:13
@niemyjski there’s this R# key you can put in to have that warn about C#6. I’m gonna use that to find all the problems, but maybe you want to have that in there as well?
Blake Niemyjski
@niemyjski
Feb 04 2016 23:19
yeah
where can I put that in
should be in the resharper settings for that project
Sander Rijken
@srijken
Feb 04 2016 23:22
would be nice if that worked
tried, it came up with them, and then they were gone
gotit
properties window of the .Portable project
UGH
the first fix it suggests is “turn on C#6 support"
and there’s no other fix
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:26
Had a discussion at work today about performance of string comparisons. Colleagues wanted to do GetHashCode to compare two long strings, but it turned out to be a bad idea: GetHashCode would need to be
...called every time and it takes about 4 times longer than comparing the strings themselves
Sander Rijken
@srijken
Feb 04 2016 23:28
makes sense
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:29
that's because string comparison and GetHashCode both need to look at every single character, but GetHashCode needs to multiply and xor things.
So, actually it is more costly and doesnt add any performance when used on few instances.
So, maybe we need more than one hashcode? Could we have a fail-fast-hash that only uses few properties?
Sander Rijken
@srijken
Feb 04 2016 23:31
no
oh wait like that
hmm
but that makes things more complicated
let’s first measure things
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:33
Yes, i dont want to prematurely optimize either.
Just saying, because GetHashCode itself is already trying to be an optimization
Eric J. Smith
@ejsmith
Feb 04 2016 23:34
problem is that you need a hash that covers the entire object in order to find matches later
Sander Rijken
@srijken
Feb 04 2016 23:35
you could in theory use a quick hash to determine that it’s different for sure
like if the event message is different.. there’s no need to do a full hashcode
Eric J. Smith
@ejsmith
Feb 04 2016 23:35
So like have 2 levels of hashes?
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:35
Yes, but those could be calculated on demand if the fail-fast-hash matched
Which would only happen rarely
Eric J. Smith
@ejsmith
Feb 04 2016 23:36
Yeah
Let's test
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:36
With on demand i mean lazily
Sander Rijken
@srijken
Feb 04 2016 23:37
well you need it right away, so I don’t get the lazily
it does save calculating them a lot though
I wonder if System.String save the hashcode internally after requesting
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:37
With lazy i mean the real hash
Sander Rijken
@srijken
Feb 04 2016 23:37
it can safely do so as it’s immutable
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:38
It didnt when i looked at the code today
Sander Rijken
@srijken
Feb 04 2016 23:38
that’s.. strange
Eric J. Smith
@ejsmith
Feb 04 2016 23:38
Could do a GetQuickHashcode that just includes the base event level properties plus includes all the data key names
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:39
But i can
Sander Rijken
@srijken
Feb 04 2016 23:39
if they do that, you have a very good change that it’s lightning fast, because most string that are generated by throw new Exception(“Somemessage”) are interned anyway
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:39
Have a look again*
Jon Skeet says its a performance / memory question, and memory wins here.
Sander Rijken
@srijken
Feb 04 2016 23:42
it’s 4 bytes.. well anyway :)
I guess there’s a lot of strings on the heap and it adds up
@niemyjski C#6 stuff has been removed, pushed
Frank Ebersoll
@frankebersoll
Feb 04 2016 23:44
Bedtime. See you!
Sander Rijken
@srijken
Feb 04 2016 23:45
can you see if it succeeds this time, as it’s also bedtime for me?
looks like
:)
Eric J. Smith
@ejsmith
Feb 04 2016 23:47
Good night guys!