Re: [Bug 11398] New: [IndexedDB] Methods that take multiple optional parameters should instead take an options object

On Wed, Jan 12, 2011 at 2:29 PM, Jeremy Orlow <jorlow@chromium.org> wrote:
> On Wed, Jan 12, 2011 at 10:13 PM, Jonas Sicking <jonas@sicking.cc> wrote:
>>
>> On Tue, Jan 11, 2011 at 2:22 AM, Jeremy Orlow <jorlow@chromium.org> wrote:
>> > On Mon, Jan 10, 2011 at 9:40 PM, Jonas Sicking <jonas@sicking.cc> wrote:
>> >>
>> >> On Tue, Dec 14, 2010 at 12:13 PM, Jeremy Orlow <jorlow@chromium.org>
>> >> wrote:
>> >> >
>> >> > On Tue, Dec 14, 2010 at 7:50 PM, Jonas Sicking <jonas@sicking.cc>
>> >> > wrote:
>> >> >>
>> >> >> On Tue, Dec 14, 2010 at 8:47 AM, Jeremy Orlow <jorlow@chromium.org>
>> >> >> wrote:
>> >> >>>
>> >> >>> Btw, I forgot to mention IDBDatabase.transaction which I definitely
>> >> >>> think should take an options object as well.
>> >> >>
>> >> >> Hmm.. I think we should make the first argument required, I actually
>> >> >> thought it was until I looked just now. I don't see what the use
>> >> >> case is for
>> >> >> opening all tables.
>> >> >
>> >> > FWIW I'm finding that the majority of the IndexedDB code I read and
>> >> > write does indeed need to lock everything.  I'm also finding that
>> >> > most of
>> >> > the code I'm writing/reading won't be helped at all by defaulting to
>> >> > READ_ONLY...
>> >> >
>> >> >>
>> >> >> In fact, it seems rather harmful that the syntax which will result
>> >> >> in
>> >> >> more lock contention is simpler than the syntax which is better
>> >> >> optimized.
>> >> >
>> >> > But you're right about this.  So, if we're trying to force users to
>> >> > write highly parallelizable code, then yes the first arg probably
>> >> > should be
>> >> > required.  But if we're trying to make IndexedDB easy to use then
>> >> > actually
>> >> > the mode should probably be changed back to defaulting to READ_WRITE.
>> >> > I know I argued for the mode default change earlier, but I'm having
>> >> > second thoughts.  We've spent so much effort making the rest of the
>> >> > API easy
>> >> > to use that having points of abrasion like this seem a bit wrong.
>> >> >  Especially if (at least in my experience) the abrasion is only going
>> >> > to
>> >> > help a limited number of cases--and probably ones where the
>> >> > developers will
>> >> > pay attention to this without us being heavy-handed.
>> >>
>> >> I think "ease of use" is different from "few characters typed". For
>> >> example it's important that the API discourages bugs, for example by
>> >> making the code easy and clear to read. Included in that is IMHO to
>> >> make it easy to make the code fast.
>> >
>> > It won't make the cost fast.  It'll make it'll allow parallel execution.
>> >  Which will only matter if a developer is trying to do multiple reads at
>> > once and you have significant latency to your backend and/or it's
>> > heavily
>> > disk bound.  Which will only be true in complex web apps--the kind where
>> > a
>> > developer is going to be more conscious of various performance
>> > bottlenecks.
>> >  In other words, most of the time, defaulting to READ_ONLY will almost
>> > certainly have no visible impact in speed.
>>
>> It also matters for the use cases of having background workers reading
>> from the same table,
>
> Workers are a pretty advanced use case.  One where I'd expect the developer
> to be mindful of something like this.
>
>> as well as any time the user opens two tabs to
>> the same page. The latter is something that I expect every web app
>> would care about.
>
> A user will generally only be using one page at a time.  The few apps that I
> can think of where this in't true would be fairly advanced use cases where
> the developer is going to need to consciously optimize their app anyway.
> I doubt that you're going to save the world more grief than you're going to
> cause them by defaulting to READ_ONLY.

It's not a matter of just being mindful. Even for advanced users, I
think it's wrong to create an API where they constantly "have to be
mindful" any time they are using an API. Every time where an API
requires people to constantly remember to take a certain action, which
implicitly create the situation where code reviewers have to keep an
eye out for misuse, I think we have failed.

We've had similar situations in gecko. For example it used to be the
case that any time you used a JS object, you had to remember to root
it as long as you were using it, and unroot when you were done. After
having forgotten a few times, we made it common practice for code
reviewers to always keep an eye out for this. When this still wasn't
working we first developed syntax which would make it easier to do the
right thing. When this still wasn't working we created static analysis
tools which ran over the source code and tried to catch misuses. When
a few errors still slipped through we finally implemented conservative
stack scanning which meant that developers no longer needed to be
mindful and the whole problem went away.

So there is no question in my mind weather defaulting to READ_WRITE
will result in people using that when they shouldn't. Even in complex
applications which will slow down because of it.

The question to me is rather weather we are ok with that slowdown
compared to saving users some typing. In my opinion it's the wrong
tradeoff.

/ Jonas

Received on Wednesday, 12 January 2011 23:49:02 UTC