[00:51] <RAOF> racarr: Hey, I'm about to do a whole lot of commenting on introduce-bufferstream.
[00:51] <RAOF> racarr: Would you prefer an IRC discussion? :)
[00:53] <racarr> RAOF: Yeah IRC sounds better :)
[00:56] <RAOF> Ok.
[00:57] <RAOF> racarr: So, first off - why create_surfaceless_buffer_stream, rather than just create_buffer_stream?
[00:58] <racarr> RAOF: I guess because I'm thinking of BufferStream as analagous to an interafce implemented by
[00:58] <racarr> Surface and Screencast
[00:58] <racarr> so create_buffer_stream is
[00:59] <racarr> mildly incorrect?
[01:00] <RAOF> That brings up point 2 - why does MirScreencast still exist? :)
[01:01] <RAOF> So, my feeling is that mir_connection_create_surfaceless_buffer_stream is mildly incorrect; among other things, a screencast is *also* a surfaceless bufferstream.
[01:02] <racarr> hmm
[01:02] <racarr> I guess the Screencast C'Tor could just return BufferStream
[01:02] <racarr> except for ABI break
[01:02] <RAOF> Yeah, that's what I was thinking.
[01:02] <racarr> which this otherwise avoids iirc
[01:03] <RAOF> Yeah, this is true.
[01:03] <racarr> I guess, right so the other thing is
[01:03] <racarr> then you would need mir_buffer_stream_release
[01:03] <racarr> and have to document the behavior when called on a buffer stream obtained
[01:03] <racarr> from a surface
[01:03] <RAOF> Yeah. Which is my point 3 :)
[01:04] <RAOF> My preference would be for *all* buffer streams to need to be released.
[01:04] <racarr> oO?
[01:04] <racarr> What is the use case for releasing a surface but not the buffer stream?
[01:05] <RAOF> Consistency in API, mostly :)
[01:06] <RAOF> But it's also vaguely sensible, if you just care about output.
[01:06] <racarr> Hmm I dont know it seems to require a lot more error states, e.g. what happens if you use surface in when buffer stream is released, etc...
[01:06] <racarr> ?
[01:06] <RAOF> Neither the surface nor the buffer stream are destroyed until both are.
[01:07] <racarr> What do you mean by its vaguely sensible
[01:07] <racarr> if you just care about output?
[01:10] <RAOF> Well, if you're doing something like a video output window you could happily release the surface and just swap the buffer stream.
[01:10] <RAOF> But I agree that's not particularly compelling, which is why I said vaguely sensible :)
[01:10] <racarr> I guess I'd be more inclined to have mir_buffer_stream_release be an error
[01:11] <racarr> if the buffer stream is owned by a surface
[01:11] <racarr> or something...not sure of the exact language
[01:11] <RAOF> Well, *that* could actually be a sensible usage.
[01:11] <racarr> ?
[01:11] <RAOF> If you're just doing say a static splash screen - create the surface, get the buffer stream, do your one and only swap, then release the buffer stream and now Mir can free some memory :)
[01:12] <RAOF> But eh.
[01:12] <RAOF> Mostly I want it for API consistency, and for when we want to associate multiple buffer streams with a single surface.
[01:13] <racarr> I am not sure I would quite see it
[01:13] <racarr> anything that requires every currently existing call call to mir_surface_release
[01:13] <racarr> literally without fail
[01:13] <racarr> to be prefixed with a call to mir_buffer_stream_release
[01:13] <racarr> has to be incorrect right?
[01:14] <racarr> anyone looking would say the surface should own the buffer stream and take care of releasing it....
[01:15] <RAOF> Well, I thought the same thing for MirSurfaceSpec; mir_surface_create() should eat the SurfaceSpec passed in.
[01:15] <racarr> ugh
[01:15] <racarr> lol
[01:15] <RAOF> But I don't think it's so unreasonable now :)
[01:15] <racarr> This is a little different though because the surface creates the buffer stream...
[01:16] <racarr> and I do think its unreasonable lol
[01:16] <racarr> floating references provide a better solution
[01:16] <racarr> but yes I guess that landed
[01:17] <duflu> racarr: a buffer stream allocator for clients might be what we need to make nested clients bypass/overlayable too
[01:17] <duflu> ... as the nested server can then allocate buffers from the system compositor for nested clients
[01:17] <RAOF> Also, you still have the problem of error states multiplying - you still have to specify what happens when you use buffer stream you got from a released surface.
[01:17] <racarr> lol
[01:17] <racarr> nah, it becomes invalid
[01:17] <racarr> I guess there should be
[01:17] <racarr> mir_buffer_stream_is_valid
[01:18] <RAOF> Yeah, that's necessary.
[01:18] <RAOF> duflu: Oooh, exactly right!
[01:18] <RAOF> duflu: That's ‘multiple buffer streams per surface’.
[01:18] <duflu> RAOF: Nah, just the nested server needs a way of allocating system-compositor buffers for nested clients
[01:18] <RAOF> Also, hello from Perth.
[01:19] <duflu> RAOF: Oh good morning
[01:19] <racarr> Hmm ok I am convinced mir screencast ctor should return MirBufferStream
[01:19] <racarr> I am convinced...surfaceless_buffer_stream could have a better name
[01:19] <duflu> racarr: minus "surfaceless" because that's kind of implied
[01:20] <duflu> Bugger. We're too efficient and landed the branch I was about to fix
[01:20] <racarr> I mean not necessarily some buffer streams are associated with surfaces
[01:20] <duflu> Another branch then
[01:20] <racarr> some aren't
[01:20] <racarr> but RAOF points out that screencast isnt associated with a surface
[01:20] <racarr> but isnt what I call a SurfacelessBufferSTream
[01:21] <duflu> racarr: Yeah there's a reason why it's called a stream in video encoding/decoding
[01:22] <racarr> RAOF: The part that I can't wrap my head around is why you should have to release a buffer stream seperately from the surface....
[01:22] <racarr> duflu: ?
[01:22] <duflu> racarr: The reason is just that nobody has thought of a better abstraction/noun than "stream"
[01:22] <racarr> oh lol
[01:22] <duflu> And maybe there isn't one
[01:22] <RAOF> racarr: You don't have to, but I feel it's a bit cleaner and consistent.
[01:23] <racarr> RAOF: Hmm
[01:23] <racarr> I guess I will mull over that one my thought is that
[01:23] <racarr> the consistency is "all streams which you allocat ehave to be released" and in this case it
[01:23] <racarr> s said that the surface allocates the stream
[01:23] <racarr> likewise I would say that in the case of attaching multiple buffer streams to surfaces, etc...
[01:23] <racarr> I guess I would expect the surface to take ownership of the stream
[01:26] <RAOF> I'm happy with that, but I'm not sure it's consistent with our current API.
[01:26] <racarr> im not sure either lol
[01:26] <racarr> wrt surface spec its different because you allocate the spec...
[01:26] <racarr> hmmm
[01:27] <RAOF> But then with your multiple-buffer-streams case you *also* allocate the buffer stream :)
[01:27] <racarr> and are we sure there really is no MirScreencast
[01:27] <racarr> I can almost imagine things like
[01:27] <racarr> mir_screencast_set_capture_rate, or mir_screencast_set_scale
[01:28] <RAOF> Maybe?
[01:28] <racarr> What if we add MObject and then MirScreencast can MIR_IMPLEMENT_INTER....
[01:28] <racarr> err....
[01:28] <RAOF> :)
[01:29] <RAOF> All nontrivial C programs are guaranteed to implement a half-arsed object system? :)
[01:29] <racarr> mm
[01:33] <racarr> it does kind of seem like set_rate could be a thing...-.-
[01:33] <RAOF> That would seem to be mir_buffer_stream_set_swap_interval?
[01:34] <racarr> hmm probably
[01:35] <racarr> Seems Screencast can probably go
[01:36] <racarr> RAOF: Ok so my plan is to change the surfaceless_buffer_stream names, drop MirScreencast, add buffer_stream_is_valid
[01:36] <racarr> do something about buffer_stream_release
[01:36] <racarr> (took notes)
[01:38] <RAOF> Hm.
[01:40] <RAOF> Does MirCursorConfiguration want to own the buffer_stream?
[01:41] <racarr> hmm
[01:42] <racarr> I dont think so at least from the client perspective
[01:42] <racarr> and thats consistent with surface spec
[01:42] <racarr> on the server side though, if the buffer stream was used as the
[01:42] <racarr> active cursor request from a surface
[01:42] <racarr> even if you release the buffer stream
[01:42] <racarr> the surface will keep around shared_ptr buffer stream and the last buffer will persist until
[01:42] <racarr> you make a new cursor request for the surface
[01:43] <racarr> which I thought was good semantics
[01:43] <racarr> it would be more obvious with unref vs release
[01:44] <RAOF> Maybe we want unref?
[01:44] <RAOF> I agree that's good semantics.
[01:46] <racarr> I kind of think we want unref, because...well I dunno, that semantic might not be entirely obvious to me as I see _release
[01:46] <racarr> right. I mean I guess its explicitly non obvious
[01:46] <racarr> because release typically means release
[01:46] <racarr> however if I saw unref
[01:46] <racarr> I would assume
[01:47] <racarr> the cursor configuration and or surface took references to the buffer stream
[01:47] <RAOF> Right.
[01:47] <RAOF> (Relatedly, Ryan is correct in saying that we need a destroy notify on our delegates)
[01:48] <racarr> indeed.
[01:49] <racarr> I'm making event ref counted in another mp...
[01:49] <RAOF> Which would happily remove the need for all our _release_sync()s.
[01:49] <racarr> maybe connection and surface should be ref counted too...
[01:49] <racarr> RAOF: ?
[01:49] <racarr> Oh I see
[01:49] <racarr> wow this reminds me of an ancient problem that _create_sync is unsafe because oyu dont have time to set delegates before
[01:49] <racarr> events can be received
[01:50] <racarr> and that delegates need to be in surface spec

[01:50] <racarr> well its unsafe in that you could miss events
[01:50] <racarr> and will miss your first focus event
[01:50] <racarr> Total tangent lol
[01:52] <racarr> ok hopefully I should get to all this tomorrow assuming I dont wake up to some sort of perfect storm of merge conflicts
[01:52] <racarr> im kind of assuming I will wake up to a perfect storm of merge conflicts but who knows
[01:53] <RAOF> Heh.
[01:58] <racarr> Ok. Off to go play some piano before dinnnnner
[01:58] <racarr> RAOF: Thanks :)
[02:34] <duflu_> RAOF: Can you check this out some time soon? ... https://code.launchpad.net/~vanvugt/mir/revert-r2165/+merge/245063
[02:36] <RAOF> Sure. I'm doing a big review sweep anyway. :)
[03:13] <racarr> whee merging sha1sums causes cascading jenkins failures again!
[04:13] <racarr> duflu: I wonder how evil MirEvent* mir_event_ref(MirEvent const*) is :p
[04:13] <racarr> I think its ok if you believe in logical constness
[04:13] <racarr> you mark the ref count mutable the compiler is happy
[04:14] <racarr> all calls mir_event_foo* return the same value
[04:14] <racarr> after mir_event_ref that they did before mir_event_ref
[04:14] <duflu> racarr: I think I abstained on that?
[04:14] <racarr> so its 'logically const' right
[04:14]  * duflu checks
[04:14] <RAOF> Uuuh, you mean MirEvent const* mir_event_ref(MirEvent const*), right?
[04:14] <racarr> duflu:  Mm :) I am just saying maybe there wont be an ABI break
[04:14] <RAOF> I'd be fine with that; you're not changing the contents of the MirEvent.
[04:14] <racarr> RAOF: Even better!
[04:14] <racarr> lol
[04:15] <racarr> hmm
[04:15] <racarr> no
[04:15] <duflu> racarr: Mutable or not, it would be poor C design to go changing things insider a const object
[04:15] <racarr> RAOF: But then mir_event_unref would have to accept a const
[04:15] <racarr> MirEvent
[04:15] <racarr> which...is less
[04:15] <racarr> more questionable
[04:16] <racarr> I dont think youc an say its logically const
[04:16] <racarr> if you destroy the object
[04:16] <racarr> lol
[04:16] <RAOF> Why does MirEvent want to be refcounted, by the way?
[04:16] <duflu> racarr: Non-const always, but just work to make the return value optional later?
[04:16] <duflu> Then no ABI break
[04:16] <racarr> duflu: Unfortunately clients only ever get MirEvent const*
[04:16] <duflu> Although this is mircommon, which we break regularly. Possibly a non-issue
[04:16] <RAOF> I don't know. I think it *is* logically const if mir_event_unref() only ever destroys an object that's no longer referenced anywhere.
[04:16] <racarr> oh
[04:16] <racarr> thats true
[04:17] <racarr> haha
[04:17] <racarr> well
[04:17] <racarr> thats an argument at least!
[04:17] <racarr> hmm
[04:17] <racarr> effectively it wants to be refcounted because it wants to be copied
[04:17] <RAOF> Because the client code wants to preserve it outside the event callback?
[04:18] <duflu> racarr: This is kind of why I was saying ABI breaks are inherited though. e.g. some changes in mircommon would break the client ABI
[04:18] <racarr> RAOF: Basically, and because
[04:18] <racarr> the client library code isnt going to access it after that
[04:18] <racarr> I dont think you gain any sort of avoid synchronization benefit from a copy
[04:18] <racarr> and ref will be most efficient
[04:18] <racarr> RAOF: Then the double controversy is because I can't add ref_count to MirEvent yet mir_event_ref currently copies the event
[04:18] <racarr> :p
[04:19] <RAOF> Well, if you only ever use it inside the event callback then you can just pass the const*.
[04:19] <RAOF> What code wants to use the event outside the callback?
[04:19] <duflu> racarr: Surely while it's not opaque the user can just copy it? :) a = *b
[04:19] <racarr> RAOF: Qt queues it up as a task on the main loop
[04:19] <racarr> duflu: That's what qtubuntu does now
[04:19] <racarr> and im trying to
[04:19] <racarr> port it
[04:19] <racarr> :)
[04:19] <RAOF> Ah, fair enough.
[04:20] <RAOF> Then I'm for mir_event_ref and mir_event_unref, both working on const*
[04:21] <racarr> mm I think so too
[04:21] <racarr> will update in morning
[05:20] <RAOF> duflu: std::atomic_bool _still_ isn't equivalent to std::atomic<bool> :)
[05:34] <anpok> RAOF: hmm it cannot find the graphics-dummy platform
[05:36]  * RAOF looks.
[05:37] <RAOF> Oh, on the mako?
[05:38] <anpok> hm there we install the packages and do not run from build location
[05:39] <anpok> RAOF: oh you meant it should =log on the client and server by default?
[05:39] <RAOF> anpok: Yes.
[05:40] <RAOF> anpok: But more; I don't think that it should be a report at all. It was just - when all you have is a ReportPattern, everything seems to be a report.
[05:40] <anpok> :)
[05:41] <RAOF> Because why would you _ever_ want to use SHARED_LIBRARY_PROBER_REPORT=lttng
[05:41] <RAOF> ?
[05:41] <anpok> RAOF: if all your reports report to lttng.. all your reports report to lttng
[05:44] <anpok> to be honest I only made that change, because I needed it for the input side
[05:45] <RAOF> Yeah.
[05:45] <RAOF> I think that short-term defaulting to log is fine.
[05:47] <RAOF> And then, longer term, we just need to drop the SharedLibraryProberReport interface entirely, move mir::logging into mircommon, and then have libraries_for_path log directly, rather than take a SharedLibraryProberReport.
[05:51] <RAOF> anpok: Oh, of course!
[05:53] <RAOF> anpok: You're missing a path separator in the as-installed probe.
[05:53] <RAOF> anpok: Would you like to fix it, or shall I?
[05:54] <RAOF> anpok: Specifically:
[05:54] <RAOF> +         {library_path() + "/server-modules/", library_path() + "/server-platform/", std::string(MIR_SERVER_PLATFORM_PATH)})
[05:54] <RAOF> should be
[05:54] <RAOF> +         {library_path() + "/server-modules/", library_path() + "/server-platform/", std::string(MIR_SERVER_PLATFORM_PATH) + "/"})
[05:55] <RAOF> Because MIR_SERVER_PLATFORM_PATH=/usr/lib/linux-armhf-gnueabi/server-platform
[05:58] <duflu> RAOF: Not equivalent, but more efficient and sufficient possibly
[05:59] <duflu> Although atomic anything is open to races. I think we're turning a blind eye to that most of the time
[06:02] <duflu> Hmm, the sensible thing to do would be to find ways to tie up loose ends by tomorrow
[06:02] <duflu> Not sure how to achieve that more than we already are :P
[06:03] <RAOF> duflu: Actually, now that I've looked a bit more, I take that back.
[06:03] <RAOF> std::atomic_bool is a typedef to std::atomic<bool>.
[06:03] <RAOF> So we're both wrong :)
[06:03] <duflu> RAOF: Regardless if you know enough about the arch it could also be typedef bool :)
[06:03] <duflu> sigactomic_t
[06:04] <duflu> sigactomic_t
[06:04] <duflu> or sig_atomic_t
[06:04] <duflu> one of them
[06:04] <RAOF> I don't think sig_atomic_t is guaranteed to be thread-atomic.
[06:05] <RAOF> And it can't be typedef bool, because there's memory barriers involved.
[06:05] <duflu> RAOF: It's a pretty safe bet if the type is within the word size
[06:05] <duflu> RAOF: Barriers?!
[06:06] <RAOF> Yes, barriers?
[06:06] <duflu> Oh I see. std::atomic is quite featured.
[06:06] <duflu> Too many features for some uses
[06:08] <duflu> RAOF: I guess not if is_lock_free()
[06:08] <RAOF> Barriers != locks.
[06:08] <RAOF> Barriers are “dear CPU+compiler, please actually read this memory”
[06:08] <duflu> RAOF: ??
[06:09] <racarr> e.g. cache invalidation
[06:09] <RAOF> Compilers are free to reorder or entirely elide reads; likewise CPUs (and, indeed, multiple levels of the memory heirarchy)
[06:11] <duflu> Interesting. I always use spinlock as my lowest level primitives but seems like barriers might also be useful
[06:12] <RAOF> duflu: Feel like a quick and easy top-approve?
[06:12] <RAOF> duflu: https://code.launchpad.net/~mir-team/mir/more-inconsistent-overrides/+merge/245068
[06:14] <RAOF> racarr: :P
[06:15] <RAOF> duflu: http://en.wikipedia.org/wiki/Memory_barrier if you're interested. In particular, memory barriers are required to correctly implement any synchronisation primitive. :)
[06:18] <duflu> RAOF: Right. Back on the original topic. If you can avoid a template instantiating type then it's obviously preferable. Some compilers at least will extern the std::atomic_foo methods
[06:18] <duflu> Or extern the instantiation
[06:18] <RAOF> The instantiation is in libstdc++, though.
[06:19] <duflu> RAOF: Yes, in some implementations. What's important is that it might be typedef'd to something that's not an instantiation, hence more efficient
[06:24] <RAOF> I'm not entirely sure how that follows?
[06:31] <duflu> RAOF: Not mentioning '<>' in the code ensures you don't slow down the compiler to check for or create instantiations
[06:31] <duflu> Unfortunately not an advantage if the library typedefs to one
[06:32] <duflu> But other C++ implementations would benefit
[06:48] <duflu> I don't remember the end of any year ever being this busy
[06:48] <racarr> The year jenkins stole christmas
[06:49] <duflu> racarr: To be fair to Jenkins, it didn't slow us down. We had already landed everything we could. It just sucked up my time landing things
[06:55] <racarr> I dunno not to give anyone a hard time because I did the same thing to some branches but I assume introduce-buffer-stream etc wouldnt have gone a full week
[06:55] <racarr> without review
[06:55] <racarr> if they had a +1 from jenkins
[06:56] <racarr> or maybe europe just doesnt like me anymore because I sleep through standup and there are only two of you guys
[06:56] <racarr> who knows
[07:00] <duflu> racarr: I know how it feels. Safe to say everyone's waiting for things of theirs to lang
[07:00] <duflu> land
[07:01] <racarr> :)
[07:01] <duflu> And I'm now involved in more code reviews than I can feasibly keep up with. So staying out of others
[07:01] <racarr> Thats how I felt towards EOD today toooo
[07:01] <racarr> Ok daily show -> bed
[07:02] <duflu> racarr: As a matter of psychological trickery, developers tend toward reviewing smaller things first
[07:02] <duflu> Not that that's a "trick" but helpful to know
[07:08] <anpok> RAOF: uh!
[08:04] <RAOF> Boo.
[08:05] <RAOF> Fewer random hard shutdowns please laptop.
[08:07] <mlankhorst> RAOF: hey did you take a look at standalone xmir yet? :P
[08:07] <RAOF> mlankhorst: What did you want me to look at again?
[08:07] <RAOF> I've been off for the last 3 days, so may have lost some state :)
[08:08] <mlankhorst> no just in general if you have looked at it or not
[08:28] <RAOF> mlankhorst: Oh, no, I've not.
[08:29] <RAOF> mlankhorst: Is there anything you'd like me to do about it?
[08:31] <mlankhorst> not in particularly, just if you could look it over
[15:05] <Mark__> hello, anyone here?
[22:17] <racarr> can I TA add-more-event-getters?
[22:17] <racarr> Happy to lose the dependency on my branches...
[22:34] <racarr> *removes MirScreencast from public API*
[22:35] <racarr> Chris pointed out last night MirBufferStream* mir_connection_create_screencast can be enough